diff --git a/.eslintrc.js b/.eslintrc.js index 44abcb66b5..6735aa3b5f 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -39,7 +39,7 @@ module.exports = { { "SwitchCase": 1 } ], "linebreak-style": [ "error", "unix" ], - "quotes": [ "error", "single" ], + "quotes": [ "error", "single", { "avoidEscape": true } ], "semi": [ "error", "always" ], "no-extra-semi": "error", "comma-dangle": [ "error", "always-multiline" ], @@ -51,6 +51,7 @@ module.exports = { "dot-notation": "error", "no-alert": "error", "no-caller": "error", + "no-constant-condition": ["error", { checkLoops: false }], "no-else-return": "error", "no-eval": "error", "no-implied-eval": "error", diff --git a/package.json b/package.json index 153a61ac62..c24b9d9ef2 100644 --- a/package.json +++ b/package.json @@ -51,6 +51,7 @@ "chai": "^3.5.0", "clamp": "^1.0.1", "classnames": "^2.2.5", + "common-tags": "^1.4.0", "copy-to-clipboard": "^3.0.5", "css-loader": "^0.26.0", "eslint": "^3.10.2", @@ -89,6 +90,7 @@ "reselect": "^2.5.1", "rimraf": "^2.5.4", "shallowequal": "^0.2.2", + "sinon": "^2.1.0", "style-loader": "^0.13.1", "text-encoding": "^0.6.4", "url": "^0.11.0", diff --git a/src/content/actions/receive-profile.js b/src/content/actions/receive-profile.js index d18e722af0..8ac628c457 100644 --- a/src/content/actions/receive-profile.js +++ b/src/content/actions/receive-profile.js @@ -1,10 +1,12 @@ // @flow +import { oneLine } from 'common-tags'; import { getProfile } from '../reducers/profile-view'; import { processProfile, unserializeProfileOfArbitraryFormat } from '../process-profile'; import { SymbolStore } from '../symbol-store'; import { symbolicateProfile } from '../symbolication'; import { decompress } from '../gz'; import { getTimeRangeIncludingAllThreads } from '../profile-data'; +import { TemporaryError } from '../errors'; import type { Action, @@ -232,24 +234,95 @@ export function receiveProfileFromWeb(profile: Profile): ThunkAction { }; } -export function errorReceivingProfileFromWeb(error: any): Action { +export function temporaryErrorReceivingProfileFromWeb(error: TemporaryError): Action { return { - type: 'ERROR_RECEIVING_PROFILE_FROM_WEB', + type: 'TEMPORARY_ERROR_RECEIVING_PROFILE_FROM_WEB', error, }; } +export function fatalErrorReceivingProfileFromWeb(error: Error): Action { + return { + type: 'FATAL_ERROR_RECEIVING_PROFILE_FROM_WEB', + error, + }; +} + +function _wait(delayMs) { + return new Promise(resolve => setTimeout(resolve, delayMs)); +} + +type FetchProfileArgs = { + url: string, + onTemporaryError: TemporaryError => void, +}; + +/** + * Tries to fetch a profile on `url`. If the profile is not found, + * `onTemporaryError` is called with an appropriate error, we wait 1 second, and + * then tries again. If we still can't find the profile after 11 tries, the + * returned promise is rejected with a fatal error. + * If we can retrieve the profile properly, the returned promise is resolved + * with the JSON.parsed profile. + */ +async function _fetchProfile({ url, onTemporaryError }: FetchProfileArgs) { + const MAX_WAIT_SECONDS = 10; + let i = 0; + + while (true) { + const response = await fetch(url); + // Case 1: successful answer. + if (response.ok) { + const json = await response.json(); + return json; + } + + // case 2: unrecoverable error. + if (response.status !== 404) { + throw new Error(oneLine` + Could not fetch the profile on remote server. + Response was: ${response.status} ${response.statusText}. + `); + } + + // case 3: 404 errors can be transient while a profile is uploaded. + + if (i++ === MAX_WAIT_SECONDS) { + // In the last iteration we don't send a temporary error because we'll + // throw an error right after the while loop. + break; + } + + onTemporaryError(new TemporaryError( + 'Profile not found on remote server.', + { count: i, total: MAX_WAIT_SECONDS + 1 } // 11 tries during 10 seconds + )); + + await _wait(1000); + } + + throw new Error(oneLine` + Could not fetch the profile on remote server: + still not found after ${MAX_WAIT_SECONDS} seconds. + `); +} + export function retrieveProfileFromWeb(hash: string): ThunkAction { - return dispatch => { + return async function (dispatch) { dispatch(waitingForProfileFromWeb()); - fetch(`https://profile-store.commondatastorage.googleapis.com/${hash}`).then(response => response.text()).then(text => { - const profile = unserializeProfileOfArbitraryFormat(text); + try { + const serializedProfile = await _fetchProfile({ + url: `https://profile-store.commondatastorage.googleapis.com/${hash}`, + onTemporaryError: e => dispatch(temporaryErrorReceivingProfileFromWeb(e)), + }); + + const profile = unserializeProfileOfArbitraryFormat(serializedProfile); if (profile === undefined) { throw new Error('Unable to parse the profile.'); } - if (window.legacyRangeFilters) { + if (typeof window !== 'undefined' && window.legacyRangeFilters) { const zeroAt = getTimeRangeIncludingAllThreads(profile).start; window.legacyRangeFilters.forEach( ({ start, end }) => dispatch({ @@ -261,10 +334,9 @@ export function retrieveProfileFromWeb(hash: string): ThunkAction { } dispatch(receiveProfileFromWeb(profile)); - - }).catch(error => { - dispatch(errorReceivingProfileFromWeb(error)); - }); + } catch (error) { + dispatch(fatalErrorReceivingProfileFromWeb(error)); + } }; } diff --git a/src/content/actions/types.js b/src/content/actions/types.js index d086aab7d4..4337a8033a 100644 --- a/src/content/actions/types.js +++ b/src/content/actions/types.js @@ -4,6 +4,7 @@ import type { Profile, Thread, ThreadIndex, IndexIntoMarkersTable, IndexIntoFunc import type { State } from '../reducers/types'; import type { GetLabel } from '../labeling-strategies'; import type { GetCategory } from '../color-categories'; +import type { TemporaryError } from '../errors'; export type ExpandedSet = Set; export type PrefixCallTreeFilter = { @@ -43,7 +44,7 @@ type ProfileSummaryAction = { type: "PROFILE_SUMMARY_COLLAPSE", threadIndex: ThreadIndex }; type ProfileAction = - { type: "FILE_NOT_FOUND", url: string } | + { type: "ROUTE_NOT_FOUND", url: string } | { type: 'CHANGE_THREAD_ORDER', threadOrder: ThreadIndex[] } | { type: 'HIDE_THREAD', threadIndex: ThreadIndex } | { type: 'SHOW_THREAD', threads: Thread[], threadIndex: ThreadIndex } | @@ -61,7 +62,8 @@ type ReceiveProfileAction = } | { type: 'DONE_SYMBOLICATING' } | { type: 'ERROR_RECEIVING_PROFILE_FROM_FILE', error: any } | - { type: 'ERROR_RECEIVING_PROFILE_FROM_WEB', error: any } | + { type: 'TEMPORARY_ERROR_RECEIVING_PROFILE_FROM_WEB', error: TemporaryError } | + { type: 'FATAL_ERROR_RECEIVING_PROFILE_FROM_WEB', error: Error } | { type: 'PROFILE_PROCESSED', profile: Profile, toWorker: true } | { type: "RECEIVE_PROFILE_FROM_ADDON", profile: Profile } | { type: "RECEIVE_PROFILE_FROM_FILE", profile: Profile } | diff --git a/src/content/containers/Root.js b/src/content/containers/Root.js index 1c41c45258..c6a3fadc44 100644 --- a/src/content/containers/Root.js +++ b/src/content/containers/Root.js @@ -8,7 +8,36 @@ import { getView } from '../reducers/app'; import { getDataSource, getHash } from '../reducers/url-state'; import URLManager from './URLManager'; +import type { State, AppViewState } from '../reducers/types'; +import type { Action } from '../actions/types'; + +const LOADING_MESSAGES = Object.freeze({ + 'from-addon': 'Retrieving profile from the gecko profiler addon...', + 'from-file': 'Reading the file and parsing the profile in it...', + 'local': 'Not implemented yet.', + 'public': 'Retrieving profile from the public profile store...', +}); + +// TODO Switch to a proper i18n library +function fewTimes(count: number) { + switch (count) { + case 1: return 'once'; + case 2: return 'twice'; + default: return `${count} times`; + } +} + +type ProfileViewProps = { + view: AppViewState, + dataSource: string, + hash: string, + retrieveProfileFromAddon: void => void, + retrieveProfileFromWeb: string => void, +}; + class ProfileViewWhenReadyImpl extends Component { + props: ProfileViewProps; + componentDidMount() { const { dataSource, hash, retrieveProfileFromAddon, retrieveProfileFromWeb } = this.props; switch (dataSource) { @@ -28,26 +57,31 @@ class ProfileViewWhenReadyImpl extends Component { render() { const { view, dataSource } = this.props; - switch (view) { + switch (view.phase) { case 'INITIALIZING': { - switch (dataSource) { - case 'none': - return ; - case 'from-addon': - return
Retrieving profile from the gecko profiler addon...
; - case 'from-file': - return
Reading the file and parsing the profile in it...
; - case 'local': - return
Not implemented yet.
; - case 'public': - return
Retrieving profile from the public profile store...
; - default: - return
View not found.
; + if (dataSource === 'none') { + return ; + } + + const message = LOADING_MESSAGES[dataSource] || 'View not found'; + let additionalMessage = null; + if (view.additionalData && view.additionalData.attempt) { + const attempt = view.additionalData.attempt; + additionalMessage = `Tried ${fewTimes(attempt.count)} out of ${attempt.total}.`; } + + return ( +
+
{ message }
+ { additionalMessage &&
{ additionalMessage }
} +
+ ); } + case 'FATAL_ERROR': + return
{"Couldn't load the profile from the store."}
; case 'PROFILE': return ; - case 'FILE_NOT_FOUND': + case 'ROUTE_NOT_FOUND': return
There is no route handler for the URL {window.location.pathname + window.location.search}
; default: return
View not found.
; @@ -56,7 +90,10 @@ class ProfileViewWhenReadyImpl extends Component { } ProfileViewWhenReadyImpl.propTypes = { - view: PropTypes.string.isRequired, + view: PropTypes.shape({ + phase: PropTypes.string.isRequired, + additionalMessage: PropTypes.object, + }).isRequired, dataSource: PropTypes.string.isRequired, hash: PropTypes.string, retrieveProfileFromAddon: PropTypes.func.isRequired, @@ -69,7 +106,13 @@ const ProfileViewWhenReady = connect(state => ({ hash: getHash(state), }), actions)(ProfileViewWhenReadyImpl); +type RootProps = { + store: Store, +}; + export default class Root extends Component { + props: RootProps; + render() { const { store } = this.props; return ( diff --git a/src/content/containers/URLManager.js b/src/content/containers/URLManager.js index 0d1298146e..af5dff627e 100644 --- a/src/content/containers/URLManager.js +++ b/src/content/containers/URLManager.js @@ -77,5 +77,5 @@ export default connect(state => ({ }), (dispatch: Dispatch) => ({ updateURLState: urlState => dispatch({ type: '@@urlenhancer/updateURLState', urlState }), urlSetupDone: () => dispatch({ type: '@@urlenhancer/urlSetupDone' }), - show404: url => dispatch({ type: 'FILE_NOT_FOUND', url }), + show404: url => dispatch({ type: 'ROUTE_NOT_FOUND', url }), }))(URLManager); diff --git a/src/content/errors.js b/src/content/errors.js new file mode 100644 index 0000000000..80c955ba91 --- /dev/null +++ b/src/content/errors.js @@ -0,0 +1,16 @@ +// @flow + +type Attempt = { + count: number, + total: number, +}; + +export class TemporaryError extends Error { + attempt: Attempt; + + constructor(message: string, attempt: Attempt) { + super(message); + this.name = 'TemporaryError'; + this.attempt = attempt; + } +} diff --git a/src/content/reducers/app.js b/src/content/reducers/app.js index b52141bfc9..7b9caf7621 100644 --- a/src/content/reducers/app.js +++ b/src/content/reducers/app.js @@ -2,16 +2,23 @@ import { combineReducers } from 'redux'; import type { Action } from '../actions/types'; -import type { State, AppState, Reducer } from './types'; +import type { State, AppState, AppViewState, Reducer } from './types'; -function view(state: string = 'INITIALIZING', action: Action) { +function view(state: AppViewState = { phase: 'INITIALIZING' }, action: Action): AppViewState { switch (action.type) { - case 'FILE_NOT_FOUND': - return 'FILE_NOT_FOUND'; + case 'TEMPORARY_ERROR_RECEIVING_PROFILE_FROM_WEB': + return { + phase: 'INITIALIZING', + additionalData: { attempt: action.error.attempt }, + }; + case 'FATAL_ERROR_RECEIVING_PROFILE_FROM_WEB': + return { phase: 'FATAL_ERROR' }; + case 'ROUTE_NOT_FOUND': + return { phase: 'ROUTE_NOT_FOUND' }; case 'RECEIVE_PROFILE_FROM_ADDON': case 'RECEIVE_PROFILE_FROM_WEB': case 'RECEIVE_PROFILE_FROM_FILE': - return 'PROFILE'; + return { phase: 'PROFILE' }; default: return state; } @@ -29,5 +36,5 @@ const appStateReducer: Reducer = combineReducers({ view, isURLSetupDon export default appStateReducer; export const getApp = (state: State): AppState => state.app; -export const getView = (state: State): string => getApp(state).view; +export const getView = (state: State): AppViewState => getApp(state).view; export const getIsURLSetupDone = (state: State): boolean => getApp(state).isURLSetupDone; diff --git a/src/content/reducers/types.js b/src/content/reducers/types.js index 85e84f782f..295d138e68 100644 --- a/src/content/reducers/types.js +++ b/src/content/reducers/types.js @@ -32,8 +32,13 @@ export type ProfileViewState = { profile: Profile, }; +export type AppViewState = { + phase: string, + additionalData?: Object, +}; + export type AppState = { - view: string, + view: AppViewState, isURLSetupDone: boolean, }; diff --git a/src/test/store/actions.js b/src/test/store/actions.js index 18a68a53f4..c49ce29947 100644 --- a/src/test/store/actions.js +++ b/src/test/store/actions.js @@ -1,5 +1,5 @@ import { assert } from 'chai'; -import { blankStore, storeWithProfile } from '../fixtures/stores'; +import { storeWithProfile } from '../fixtures/stores'; import * as ProfileViewSelectors from '../../content/reducers/profile-view'; import * as TimelineSelectors from '../../content/reducers/timeline-view'; import * as UrlStateSelectors from '../../content/reducers/url-state'; @@ -20,25 +20,10 @@ import { changeFlameChartColorStrategy, changeTimelineExpandedThread, } from '../../content/actions/timeline'; -import { receiveProfileFromAddon } from '../../content/actions/receive-profile'; import { getCategoryByImplementation } from '../../content/color-categories'; const { selectedThreadSelectors } = ProfileViewSelectors; -const profile = require('../fixtures/profiles/profile-2d-canvas.json'); - -describe('actions/profile', function () { - it('can take a profile from an add-on and save it to state', function () { - const store = blankStore(); - - const initialProfile = ProfileViewSelectors.getProfile(store.getState()); - assert.isOk(initialProfile, 'A blank profile initially exists'); - assert.lengthOf(initialProfile.threads, 0, 'The blank profile contains no data'); - store.dispatch(receiveProfileFromAddon(profile)); - assert.strictEqual(ProfileViewSelectors.getProfile(store.getState()), profile, 'The passed-in profile is saved in state.'); - }); -}); - describe('selectors/getStackTimingByDepthForFlameChart', function () { /** * This table shows off how a flame chart gets filtered to JS only, where the number is diff --git a/src/test/store/receive-profile.js b/src/test/store/receive-profile.js new file mode 100644 index 0000000000..7b5cbb8eae --- /dev/null +++ b/src/test/store/receive-profile.js @@ -0,0 +1,139 @@ +import { assert } from 'chai'; +import sinon from 'sinon'; +import { blankStore } from '../fixtures/stores'; +import * as ProfileViewSelectors from '../../content/reducers/profile-view'; +import { getView } from '../../content/reducers/app'; +import { receiveProfileFromAddon, retrieveProfileFromWeb } from '../../content/actions/receive-profile'; + +import preprocessedProfile from '../fixtures/profiles/profile-2d-canvas.json'; +import exampleProfile from '../fixtures/profiles/example-profile'; + +describe('actions/receive-profile', function () { + describe('receiveProfileFromAddon', function () { + it('can take a profile from an addon and save it to state', function () { + const store = blankStore(); + + const initialProfile = ProfileViewSelectors.getProfile(store.getState()); + assert.ok(initialProfile, 'A blank profile initially exists'); + assert.lengthOf(initialProfile.threads, 0, 'The blank profile contains no data'); + store.dispatch(receiveProfileFromAddon(preprocessedProfile)); + assert.strictEqual(ProfileViewSelectors.getProfile(store.getState()), preprocessedProfile, 'The passed in profile is saved in state.'); + }); + }); + + describe('retrieveProfileFromWeb', function () { + const fetch404Response = { ok: false, status: 404 }; + const fetch500Response = { ok: false, status: 500 }; + const fetch200Response = { + ok: true, status: 200, + json: () => Promise.resolve(exampleProfile), + }; + + beforeEach(function () { + // The stub makes it easy to return different values for different + // arguments. Here we define the default return value because there is no + // argument specified. + global.fetch = sinon.stub(); + global.fetch.resolves(fetch404Response); + + sinon.stub(global, 'setTimeout').yieldsAsync(); // will call its argument asynchronously + }); + + afterEach(function () { + delete global.fetch; + global.setTimeout.restore(); + }); + + /** + * This function allows to observe all state changes in a Redux store while + * something's going on. + * @param {ReduxStore} store + * @param {() => Promise} func Process that will be started while + * observing the store. + * @returns {Promise} All states that happened while waiting for + * the end of func. + */ + async function observeStoreStateChanges(store, func) { + const states = []; + const unsubscribe = store.subscribe(() => { + states.push(store.getState()); + }); + + await func(); + + unsubscribe(); + return states; + } + + it('can retrieve a profile from the web and save it to state', async function () { + const hash = 'c5e53f9ab6aecef926d4be68c84f2de550e2ac2f'; + const expectedUrl = `https://profile-store.commondatastorage.googleapis.com/${hash}`; + global.fetch.withArgs(expectedUrl).resolves(fetch200Response); + + const store = blankStore(); + await store.dispatch(retrieveProfileFromWeb(hash)); + + const state = store.getState(); + assert.deepEqual(getView(state), { phase: 'PROFILE' }); + assert.deepEqual(ProfileViewSelectors.getDisplayRange(state), { start: 0, end: 1007 }); + assert.deepEqual(ProfileViewSelectors.getThreadOrder(state), [0, 2, 1]); // 1 is last because it's the Compositor thread + assert.lengthOf(ProfileViewSelectors.getProfile(state).threads, 3); // not empty + }); + + it('requests several times in case of 404', async function () { + const hash = 'c5e53f9ab6aecef926d4be68c84f2de550e2ac2f'; + const expectedUrl = `https://profile-store.commondatastorage.googleapis.com/${hash}`; + // The first call will still be a 404 -- remember, it's the default return value. + global.fetch.withArgs(expectedUrl).onSecondCall().resolves(fetch200Response); + + const store = blankStore(); + const views = (await observeStoreStateChanges( + store, + () => store.dispatch(retrieveProfileFromWeb(hash)) + )).map(state => getView(state)); + + assert.deepEqual( + views, + [ + { phase: 'INITIALIZING' }, + { phase: 'INITIALIZING', additionalData: { attempt: { count: 1, total: 11 }}}, + { phase: 'PROFILE' }, + ] + ); + + const state = store.getState(); + assert.deepEqual(ProfileViewSelectors.getDisplayRange(state), { start: 0, end: 1007 }); + assert.deepEqual(ProfileViewSelectors.getThreadOrder(state), [0, 2, 1]); // 1 is last because it's the Compositor thread + assert.lengthOf(ProfileViewSelectors.getProfile(state).threads, 3); // not empty + }); + + it('fails in case the profile cannot be found after several tries', async function () { + const hash = 'c5e53f9ab6aecef926d4be68c84f2de550e2ac2f'; + const store = blankStore(); + const views = (await observeStoreStateChanges( + store, + () => store.dispatch(retrieveProfileFromWeb(hash)) + )).map(state => getView(state)); + + const steps = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; + + assert.deepEqual( + views, + [ + { phase: 'INITIALIZING' }, + ...steps.map(step => ({ phase: 'INITIALIZING', additionalData: { attempt: { count: step, total: 11 }}})), + { phase: 'FATAL_ERROR' }, + ] + ); + }); + + it('fails in case the fetch returns a server error', async function () { + const hash = 'c5e53f9ab6aecef926d4be68c84f2de550e2ac2f'; + global.fetch.resolves(fetch500Response); + + const store = blankStore(); + await store.dispatch(retrieveProfileFromWeb(hash)); + assert.deepEqual(getView(store.getState()), { phase: 'FATAL_ERROR' }); + }); + }); +}); diff --git a/webpack.config.js b/webpack.config.js index 79458fbd1d..a616e6007e 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -63,6 +63,9 @@ const baseConfig = { loader: 'file-loader', }], }, + node: { + process: false, + }, }; if (process.env.NODE_ENV === 'development') {