Merge pull request #3436 from benjiwheeler/project-state-error

benjiwheeler · web-flow · commit 4fd0019d5dda · 2018-10-24T11:17:48.000-04:00
use project state error for project state related errors
diff --git a/src/containers/gui.jsx b/src/containers/gui.jsx
@@ -9,6 +9,7 @@ import {defineMessages, injectIntl, intlShape} from 'react-intl';
 import ErrorBoundaryHOC from '../lib/error-boundary-hoc.jsx';
 import {openExtensionLibrary} from '../reducers/modals';
 import {
+    getIsError,
     getIsShowingProject
 } from '../reducers/project-state';
 import {setProjectTitle} from '../reducers/project-title';
@@ -63,16 +64,16 @@ class GUI extends React.Component {
         }
     }
     render () {
-        if (this.props.loadingError) {
+        if (this.props.isError) {
             throw new Error(
-                `Failed to load project from server [id=${window.location.hash}]: ${this.props.errorMessage}`);
+                `Error in Scratch GUI [location=${window.location}]: ${this.props.error}`);
         }
         const {
             /* eslint-disable no-unused-vars */
             assetHost,
-            errorMessage,
+            error,
             hideIntro,
-            loadingError,
+            isError,
             isShowingProject,
             onUpdateProjectId,
             onUpdateReduxProjectTitle,
@@ -100,14 +101,14 @@ class GUI extends React.Component {
 GUI.propTypes = {
     assetHost: PropTypes.string,
     children: PropTypes.node,
-    errorMessage: PropTypes.string,
+    error: PropTypes.oneOfType([PropTypes.object, PropTypes.string]),
     fetchingProject: PropTypes.bool,
     hideIntro: PropTypes.bool,
     importInfoVisible: PropTypes.bool,
     intl: intlShape,
+    isError: PropTypes.bool,
     isLoading: PropTypes.bool,
     isShowingProject: PropTypes.bool,
-    loadingError: PropTypes.bool,
     loadingStateVisible: PropTypes.bool,
     onSeeCommunity: PropTypes.func,
     onUpdateProjectId: PropTypes.func,
@@ -135,7 +136,9 @@ const mapStateToProps = (state, ownProps) => {
         connectionModalVisible: state.scratchGui.modals.connectionModal,
         costumeLibraryVisible: state.scratchGui.modals.costumeLibrary,
         costumesTabVisible: state.scratchGui.editorTab.activeTabIndex === COSTUMES_TAB_INDEX,
+        error: state.scratchGui.projectState.error,
         importInfoVisible: state.scratchGui.modals.importInfo,
+        isError: getIsError(loadingState),
         isPlayerOnly: state.scratchGui.mode.isPlayerOnly,
         isRtl: state.locales.isRtl,
         isShowingProject: getIsShowingProject(loadingState),
diff --git a/src/lib/project-fetcher-hoc.jsx b/src/lib/project-fetcher-hoc.jsx
@@ -8,6 +8,7 @@ import {
     LoadingStates,
     defaultProjectId,
     getIsFetchingWithId,
+    onError,
     onFetchedProjectData,
     setProjectId
 } from '../reducers/project-state';
@@ -74,18 +75,20 @@ const ProjectFetcherHOC = function (WrappedComponent) {
                     }
                 })
                 .catch(err => {
+                    this.props.onError(err);
                     log.error(err);
                 });
         }
         render () {
             const {
                 /* eslint-disable no-unused-vars */
                 assetHost,
-                onFetchedProjectData: onFetchedProjectDataProp,
                 intl,
+                loadingState,
+                onError: onErrorProp,
+                onFetchedProjectData: onFetchedProjectDataProp,
                 projectHost,
                 projectId,
-                loadingState,
                 reduxProjectId,
                 setProjectId: setProjectIdProp,
                 /* eslint-enable no-unused-vars */
@@ -106,6 +109,7 @@ const ProjectFetcherHOC = function (WrappedComponent) {
         intl: intlShape.isRequired,
         isFetchingWithId: PropTypes.bool,
         loadingState: PropTypes.oneOf(LoadingStates),
+        onError: PropTypes.func,
         onFetchedProjectData: PropTypes.func,
         projectHost: PropTypes.string,
         projectId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
@@ -123,6 +127,7 @@ const ProjectFetcherHOC = function (WrappedComponent) {
         reduxProjectId: state.scratchGui.projectState.projectId
     });
     const mapDispatchToProps = dispatch => ({
+        onError: error => dispatch(onError(error)),
         onFetchedProjectData: (projectData, loadingState) =>
             dispatch(onFetchedProjectData(projectData, loadingState)),
         setProjectId: projectId => dispatch(setProjectId(projectId))
diff --git a/src/lib/project-saver-hoc.jsx b/src/lib/project-saver-hoc.jsx
@@ -90,14 +90,14 @@ const ProjectSaverHOC = function (WrappedComponent) {
             const {
                 /* eslint-disable no-unused-vars */
                 createProject: createProjectProp,
-                onCreated: onCreatedProp,
-                onUpdated: onUpdatedProp,
-                onError: onErrorProp,
                 isCreating: isCreatingProp,
                 isShowingWithId: isShowingWithIdProp,
                 isShowingWithoutId: isShowingWithoutIdProp,
                 isUpdating: isUpdatingProp,
                 loadingState,
+                onCreated: onCreatedProp,
+                onError: onErrorProp,
+                onUpdated: onUpdatedProp,
                 reduxProjectId,
                 saveProject: saveProjectProp,
                 /* eslint-enable no-unused-vars */
@@ -141,7 +141,7 @@ const ProjectSaverHOC = function (WrappedComponent) {
         createProject: () => dispatch(createProject()),
         onCreated: projectId => dispatch(onCreated(projectId)),
         onUpdated: (projectId, loadingState) => dispatch(onUpdated(projectId, loadingState)),
-        onError: errStr => dispatch(onError(errStr)),
+        onError: error => dispatch(onError(error)),
         saveProject: () => dispatch(saveProject())
     });
     // Allow incoming props to override redux-provided props. Used to mock in tests.
diff --git a/src/lib/vm-manager-hoc.jsx b/src/lib/vm-manager-hoc.jsx
@@ -8,6 +8,7 @@ import AudioEngine from 'scratch-audio';
 
 import {
     LoadingStates,
+    onError,
     onLoadedProject,
     getIsLoadingWithId
 } from '../reducers/project-state';
@@ -24,10 +25,6 @@ const vmManagerHOC = function (WrappedComponent) {
             bindAll(this, [
                 'loadProject'
             ]);
-            this.state = {
-                loadingError: false,
-                errorMessage: ''
-            };
         }
         componentDidMount () {
             if (this.props.vm.initialized) return;
@@ -51,29 +48,26 @@ const vmManagerHOC = function (WrappedComponent) {
                     this.props.onLoadedProject(this.props.loadingState, this.props.canSave);
                 })
                 .catch(e => {
-                    // Need to catch this error and update component state so that
-                    // error page gets rendered if project failed to load
-                    this.setState({loadingError: true, errorMessage: e});
+                    this.props.onError(e);
                 });
         }
         render () {
             const {
                 /* eslint-disable no-unused-vars */
                 fontsLoaded,
+                loadingState,
+                onError: onErrorProp,
                 onLoadedProject: onLoadedProjectProp,
                 projectData,
                 projectId,
-                loadingState,
                 /* eslint-enable no-unused-vars */
                 isLoadingWithId: isLoadingWithIdProp,
                 vm,
                 ...componentProps
             } = this.props;
             return (
                 <WrappedComponent
-                    errorMessage={this.state.errorMessage}
                     isLoading={isLoadingWithIdProp}
-                    loadingError={this.state.loadingError}
                     vm={vm}
                     {...componentProps}
                 />
@@ -86,6 +80,7 @@ const vmManagerHOC = function (WrappedComponent) {
         fontsLoaded: PropTypes.bool,
         isLoadingWithId: PropTypes.bool,
         loadingState: PropTypes.oneOf(LoadingStates),
+        onError: PropTypes.func,
         onLoadedProject: PropTypes.func,
         projectData: PropTypes.oneOfType([PropTypes.object, PropTypes.string]),
         projectId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
@@ -103,6 +98,7 @@ const vmManagerHOC = function (WrappedComponent) {
     };
 
     const mapDispatchToProps = dispatch => ({
+        onError: error => dispatch(onError(error)),
         onLoadedProject: (loadingState, canSave) =>
             dispatch(onLoadedProject(loadingState, canSave))
     });
diff --git a/src/reducers/project-state.js b/src/reducers/project-state.js
@@ -65,9 +65,12 @@ const getIsShowingWithId = loadingState => (
 const getIsShowingWithoutId = loadingState => (
     loadingState === LoadingState.SHOWING_WITHOUT_ID
 );
+const getIsError = loadingState => (
+    loadingState === LoadingState.ERROR
+);
 
 const initialState = {
-    errStr: null,
+    error: null,
     projectData: null,
     projectId: null,
     loadingState: LoadingState.NOT_LOADED
@@ -221,7 +224,7 @@ const reducer = function (state, action) {
         ].includes(state.loadingState)) {
             return Object.assign({}, state, {
                 loadingState: LoadingState.ERROR,
-                errStr: action.errStr
+                error: action.error
             });
         }
         return state;
@@ -295,9 +298,9 @@ const onUpdated = loadingState => {
     }
 };
 
-const onError = errStr => ({
+const onError = error => ({
     type: GO_TO_ERROR_STATE,
-    errStr: errStr
+    error: error
 });
 
 const setProjectId = id => ({
@@ -326,6 +329,7 @@ export {
     createProject,
     defaultProjectId,
     getIsCreating,
+    getIsError,
     getIsFetchingWithoutId,
     getIsFetchingWithId,
     getIsLoadingWithId,
diff --git a/test/unit/reducers/project-state-reducer.test.js b/test/unit/reducers/project-state-reducer.test.js
@@ -17,7 +17,7 @@ test('initialState', () => {
     let defaultState;
     /* projectStateReducer(state, action) */
     expect(projectStateReducer(defaultState, {type: 'anything'})).toBeDefined();
-    expect(projectStateReducer(defaultState, {type: 'anything'}).errStr).toBe(null);
+    expect(projectStateReducer(defaultState, {type: 'anything'}).error).toBe(null);
     expect(projectStateReducer(defaultState, {type: 'anything'}).projectData).toBe(null);
     expect(projectStateReducer(defaultState, {type: 'anything'}).projectId).toBe(null);
     expect(projectStateReducer(defaultState, {type: 'anything'}).loadingState).toBe(LoadingState.NOT_LOADED);
@@ -229,23 +229,23 @@ test('onError from various states should show error', () => {
     ];
     for (const startState of startStates) {
         const initialState = {
-            errStr: null,
+            error: null,
             loadingState: startState
         };
-        const action = onError('Error string');
+        const action = onError({message: 'Error string'});
         const resultState = projectStateReducer(initialState, action);
         expect(resultState.loadingState).toBe(LoadingState.ERROR);
-        expect(resultState.errStr).toBe('Error string');
+        expect(resultState.error).toEqual({message: 'Error string'});
     }
 });
 
 test('onError from showing project should show error', () => {
     const initialState = {
-        errStr: null,
+        error: null,
         loadingState: LoadingState.FETCHING_WITH_ID
     };
-    const action = onError('Error string');
+    const action = onError({message: 'Error string'});
     const resultState = projectStateReducer(initialState, action);
     expect(resultState.loadingState).toBe(LoadingState.ERROR);
-    expect(resultState.errStr).toBe('Error string');
+    expect(resultState.error).toEqual({message: 'Error string'});
 });
