Open nathanielrindlaub opened 4 months ago
Also worth trying to improve ErrorAlerts.js
- in particular it feels onerous and redundant to create error dismissing actions and reducers for each type of error. e.g.:
dismissUploadError: (state, { payload }) => {
const index = payload;
state.loadingStates.upload.errors.splice(index, 1);
},
There must be a way to abstract that a bit, maybe by adding an extraReducers
case to each slice to handle a more generic dismissError
kind of action?
And make sure all ErrorAlerts
are wired up while we're at it...
We need to be able to support consuming & storing GraphQL errors produced by the API in our application state, as well as errors thrown internally on the client side (for example, if we lose internet connection during a bulk upload and need to notify the user that the upload was aborted).
For the most part, I think we have good support for the GraphQL errors arrays and those are the most common, but we have poor and inconsistent handling when throwing
new Error()
's internally. Issues and observations so far I've noted are:errors
array. That's fine (I think), but worth noting that even is some successful mutations or queries occurred anderrors.length > 0
but there are also some data in thedata
part of the payload, we aren't able to do anything with that data unless we change the errorPolicy toerrorPolicy: 'all'
. So for now, if anyerrors
are returned from the API, graphql-request throws them, thecall()
function re-throws them as well, and then they enter thecatch
blocks of whatever Thunk made the request.ErrorAlerts.js
expects errors in state to be arrays, but it's not clear that's always the case for errors thrown on the client-sideError
class instances into Redux action creators e.g.uploadFailure(new Error('error message'))
. So we need to figure out a way to normalize/coerce client-side into GraphQLerrors
arrays before they get included as payloads in dispatched actions. Something along these lines in Thunk catch blocks perhaps?Related: https://github.com/tnc-ca-geo/animl-api/issues/147
It also might be worth looking into abandoning graphql-request for Apollo-client. Apollo-client has built in handling for network errors, but I believe that refers to server-side network errors, not client side.