From 9abb3dcad9ce750e9e35257c2d5f51f31546068c Mon Sep 17 00:00:00 2001 From: Tom Hacohen Date: Mon, 18 Nov 2019 12:41:36 +0200 Subject: [PATCH] Make parse errors non-fatal Before this patch, parse errors would make the whole app stop and show an error. Now we handle them more gracefully by showing the parsing errors in a non-fatal way. This was implemented in a hacky way, and will be changed once the web app is refactored to better resemble the iOS app. Fixes #48. --- src/App.tsx | 52 +++++++++++++++++++++++++++++++++++---- src/journal-processors.ts | 24 ++++++++++++++++-- src/store/actions.ts | 14 +++++++++++ src/store/construct.ts | 4 ++- src/store/reducers.ts | 13 ++++++++++ 5 files changed, 99 insertions(+), 8 deletions(-) diff --git a/src/App.tsx b/src/App.tsx index 25edbdd..2cedf28 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1,4 +1,5 @@ import * as React from 'react'; +import { List as ImmutableList } from 'immutable'; import { connect } from 'react-redux'; import { withRouter } from 'react-router'; import { BrowserRouter } from 'react-router-dom'; @@ -10,13 +11,18 @@ import AppBar from '@material-ui/core/AppBar'; import Toolbar from '@material-ui/core/Toolbar'; import Drawer from '@material-ui/core/Drawer'; import IconButton from '@material-ui/core/IconButton'; +import Badge from '@material-ui/core/Badge'; import NavigationMenu from '@material-ui/icons/Menu'; import NavigationBack from '@material-ui/icons/ArrowBack'; import NavigationRefresh from '@material-ui/icons/Refresh'; +import ErrorsIcon from '@material-ui/icons/Error'; import './App.css'; +import ConfirmationDialog from './widgets/ConfirmationDialog'; +import PrettyError from './widgets/PrettyError'; +import { List, ListItem } from './widgets/List'; import withSpin from './widgets/withSpin'; import ErrorBoundary from './components/ErrorBoundary'; import SideMenu from './SideMenu'; @@ -160,17 +166,19 @@ const IconRefreshWithSpin = withSpin(NavigationRefresh); class App extends React.PureComponent { public state: { drawerOpen: boolean; + errorsDialog: boolean; }; public props: { credentials: store.CredentialsType; entries: store.EntriesType; fetchCount: number; + errors: ImmutableList; }; constructor(props: any) { super(props); - this.state = { drawerOpen: false }; + this.state = { drawerOpen: false, errorsDialog: false }; this.toggleDrawer = this.toggleDrawer.bind(this); this.closeDrawer = this.closeDrawer.bind(this); @@ -180,6 +188,7 @@ class App extends React.PureComponent { public render() { const credentials = (this.props.credentials) ? this.props.credentials.value : null; + const errors = this.props.errors; const fetching = this.props.fetchCount > 0; const styles = { @@ -197,11 +206,43 @@ class App extends React.PureComponent { } iconElementRight={ - - - } - + <> + {(errors.size > 0) && ( + this.setState({ errorsDialog: true })} title="Parse Errors"> + + + + + )} + + + + + } /> + this.setState({ errorsDialog: false })} + onOk={() => this.setState({ errorsDialog: false })} + > +

+ This should not happen, please contact developers! +

+ + {errors.map((error, index) => ( + (window as any).navigator.clipboard.writeText(`${error.message}\n\n${error.stack}`)} + > + + + ))} + + + { credentials: credentialsSelector(state), entries: state.cache.entries, fetchCount: state.fetchCount, + errors: state.errors, }; }; diff --git a/src/journal-processors.ts b/src/journal-processors.ts index ac32d75..5e573c1 100644 --- a/src/journal-processors.ts +++ b/src/journal-processors.ts @@ -3,6 +3,8 @@ import { List } from 'immutable'; import * as ICAL from 'ical.js'; import { EventType, ContactType, TaskType } from './pim-types'; +import { store } from './store'; +import { addError } from './store/actions'; import * as EteSync from 'etesync'; @@ -11,7 +13,16 @@ export function syncEntriesToItemMap( const items = base; entries.forEach((syncEntry) => { - const comp = new ContactType(new ICAL.Component(ICAL.parse(syncEntry.content))); + // FIXME: this is a terrible hack to handle parsing errors + let parsed; + try { + parsed = ICAL.parse(syncEntry.content); + } catch (e) { + e.message = `${e.message}\nWhile processing: ${syncEntry.content}`; + store.dispatch(addError(undefined as any, e)); + return; + } + const comp = new ContactType(new ICAL.Component(parsed)); // FIXME:Hack (comp as any).journalUid = collection.uid; @@ -57,7 +68,16 @@ function syncEntriesToCalendarItemMap( const color = colorIntToHtml(collection.color); entries.forEach((syncEntry) => { - const comp = ItemType.fromVCalendar(new ICAL.Component(ICAL.parse(syncEntry.content))); + // FIXME: this is a terrible hack to handle parsing errors + let parsed; + try { + parsed = ICAL.parse(syncEntry.content); + } catch (e) { + e.message = `${e.message}\nWhile processing: ${syncEntry.content}`; + store.dispatch(addError(undefined as any, e)); + return; + } + const comp = ItemType.fromVCalendar(new ICAL.Component(parsed)); if (comp === null) { return; diff --git a/src/store/actions.ts b/src/store/actions.ts index 3677620..cad9781 100644 --- a/src/store/actions.ts +++ b/src/store/actions.ts @@ -185,6 +185,20 @@ export function fetchAll(etesync: CredentialsData, currentEntries: EntriesType) }; } +export const addError = createAction( + 'ADD_ERRORS', + (_etesync: CredentialsData, error: Error) => { + return error; + } +); + +export const clearErros = createAction( + 'CLEAR_ERRORS', + (_etesync: CredentialsData) => { + return true; + } +); + // FIXME: Move the rest to their own file export const setSettings = createAction( 'SET_SETTINGS', diff --git a/src/store/construct.ts b/src/store/construct.ts index bb306af..17a7508 100644 --- a/src/store/construct.ts +++ b/src/store/construct.ts @@ -9,7 +9,7 @@ import * as EteSync from 'etesync'; import { JournalsData, FetchType, EntriesData, EntriesFetchRecord, UserInfoData, JournalsFetchRecord, UserInfoFetchRecord, CredentialsTypeRemote, JournalsType, EntriesType, UserInfoType, SettingsType, - fetchCount, journals, entries, credentials, userInfo, settingsReducer, encryptionKeyReducer, + fetchCount, journals, entries, credentials, userInfo, settingsReducer, encryptionKeyReducer, errorsReducer, } from './reducers'; export interface StoreState { @@ -22,6 +22,7 @@ export interface StoreState { entries: EntriesType; userInfo: UserInfoType; }; + errors: List; } const settingsPersistConfig = { @@ -160,6 +161,7 @@ const reducers = combineReducers({ journals, userInfo, })), + errors: errorsReducer, }); export default reducers; diff --git a/src/store/reducers.ts b/src/store/reducers.ts index 0936748..9e8cbd8 100644 --- a/src/store/reducers.ts +++ b/src/store/reducers.ts @@ -277,6 +277,19 @@ export const fetchCount = handleAction( 0 ); +export const errorsReducer = handleActions( + { + [actions.addError.toString()]: (state: List, action: Action) => { + return state.push(action.payload); + }, + [actions.clearErros.toString()]: (state: List, _action: Action) => { + return state.clear(); + }, + }, + List([]) +); + + // FIXME Move all the below (potentially the fetchCount ones too) to their own file export interface SettingsType { locale: string;