Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DataStore fails to use custom errorHandler in some cases #8891

Open
3 tasks done
applyinnovations opened this issue Sep 14, 2021 · 11 comments
Open
3 tasks done

DataStore fails to use custom errorHandler in some cases #8891

applyinnovations opened this issue Sep 14, 2021 · 11 comments
Labels
DataStore Related to DataStore category feature-request Request a new feature needs-discussion Used for internal discussions

Comments

@applyinnovations
Copy link

applyinnovations commented Sep 14, 2021

Before opening, please confirm:

JavaScript Framework

React, React Native

Amplify APIs

DataStore

Amplify Categories

api

Environment information

  System:
    OS: macOS 11.5.2
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 151.64 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.2.0 - /usr/local/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 7.21.1 - ~/Projects/focapp/node_modules/.bin/npm
  Browsers:
    Chrome: 93.0.4577.63
    Safari: 14.1.2
  npmPackages:
    @babel/core: 7.9.0 => 7.9.0 (7.15.0, 7.12.3)
    @babel/plugin-proposal-class-properties: 7.8.3 => 7.14.5 (7.12.13, 7.12.1)
    @babel/plugin-proposal-decorators: 7.8.3 => 7.8.3 (7.14.5, 7.12.1)
    @babel/plugin-transform-react-constant-elements: 7.9.0 => 7.9.0 (7.14.5)
    @babel/plugin-transform-react-inline-elements: 7.9.0 => 7.9.0 
    @babel/plugin-transform-runtime: ^7.12.10 => 7.15.0 (7.12.1)
    @babel/preset-env: 7.9.0 => 7.15.0 (7.12.17, 7.12.1)
    @babel/preset-flow: 7.9.0 => 7.9.0 
    @babel/preset-react: 7.9.4 => 7.9.4 (7.14.5, 7.12.1)
    @babel/preset-typescript: 7.9.0 => 7.12.17 (7.12.1)
    @babel/register: 7.9.0 => 7.9.0 (7.15.3)
    @geist-ui/react: ^2.2.0 => 2.2.0 
    aws-amplify: 4.2.5 => 4.2.5 
    aws-appsync-auth-link: 3.0.4 => 3.0.4 (3.0.6)
    aws-appsync-subscription-link: 3.0.6 => 3.0.6 
    babel-plugin-dynamic-import-node: 2.3.0 => 2.3.3 
    babel-plugin-graphql-tag: 2.5.0 => 2.5.0 
    babel-plugin-idx: 2.4.0 => 2.4.0 
    babel-plugin-module-resolver: 4.1.0 => 4.1.0 (3.2.0)
    graphql: 15.5.1 => 15.5.1 (14.0.0, 14.7.0)
    graphql-assert-transformer: ^1.0.2 => 1.0.2 
    lodash: 4.17.21 => 4.17.21 
    prettier: 2.3.1 => 2.3.1 
    typescript: 4.3.5 => 4.0.8 

Describe the bug

DataStore uses default errorHandler for some errors, and uses my custom error handler for other errors.

I have implemented a custom error handler by calling DataStore.configure in my app entry point (App.tsx).

When submitting a DataStore.save() with a model containing invalid data:
Our AppSync API returns an error to the DataStore client and our custom error handler is used.

When submitting a DataStore.save() with an empty model:
DataStore encounters the error Error: Field name is required and uses the default error handler. Resulting in the following being logged to the console:
[WARN] 38:37.438 DataStore - failed to execute errorHandler – Error: Field name is required

Expected behavior

The expected behaviour is to catch/route all errors to my custom errorHandler

Reproduction steps

  1. Setup DataStore with AppSync API
  2. Add custom model to schema containing type Model { number: Int }
  3. Call DataStore.configure({ errorHandler: (error) => alert(Custom handler ${error.message}) });
  4. Call DataStore.start()
  5. Call DataStore.save(new Model({ number: 1 })) -> no error
  6. Call DataStore.save(new Model({ number: "not a number" })) -> custom error handler
  7. Call DataStore.save(new Model({})) -> default error handler

Code Snippet

DataStore.configure({
  errorHandler: (error) => {
    console.log(error);
    setToast({
      delay: 5000,
      text: error.message,
      type: 'error',
    });
  },
});

// empty input results in default errorHandler
DataStore.save(new Model({}));

// other test cases results in custom errorHandler
DataStore.save(new Model({ number: "not a number" }));

Log output

[Log] [DEBUG] 01:17.727 DataStore - Attempting mutation with authMode: AMAZON_COGNITO_USER_POOLS
[Log] [DEBUG] 01:17.727 Util -  attempt #1 with this vars: ["Category","Create","{\"id\":\"54e566d0-5630-486f-af22-649e3255b26d\"}","{}",null,null,{"data":"{\"id\":\"54e566d0-5630-486f-af22-649e3255b26d\"}","modelId":"54e566d0-5630-486f-af22-649e3255b26d","model":"Category","operation":"Create","condition":"{}","id":"01FFHAV6SSZQ7XYQVDW8BQ2CQV"}]
[Log] [DEBUG] 01:17.729 AuthClass - Getting current session
[Log] [DEBUG] 01:17.730 AuthClass - Getting the session from this user: – CognitoUser {username: "+61XXXXXXXXX", pool: CognitoUserPool, Session: null, …}
CognitoUser {username: "+61XXXXXXXXX", pool: CognitoUserPool, Session: null, client: Client, signInUserSession: CognitoUserSession, …}CognitoUser
[Log] [DEBUG] 01:17.731 AuthClass - Succeed to get the user session – CognitoUserSession {idToken: CognitoIdToken, refreshToken: CognitoRefreshToken, accessToken: CognitoAccessToken, …}
CognitoUserSession {idToken: CognitoIdToken, refreshToken: CognitoRefreshToken, accessToken: CognitoAccessToken, clockDrift: -1, getIdToken: function, …}CognitoUserSession
[Log] [DEBUG] 01:17.731 RestClient - POST – "https://sn4fnspmfbdm5dwl5okhzqigce.appsync-api.ap-southeast-2.amazonaws.com/graphql"
[Warning] [WARN] 01:17.879 DataStore - failed to execute errorHandler – Error: Field name is required
Error: Field name is required
[Log] [DEBUG] 01:17.880 DataStore - Mutation sent successfully with authMode: AMAZON_COGNITO_USER_POOLS
[Log] [DEBUG] 01:17.880 DataStore - done retrying
[Log] [DEBUG] 01:34.765 AWSAppSyncRealTimeProvider - subscription message from AWS AppSync RealTime: {"type":"ka"}
[Log] [DEBUG] 01:34.765 AWSAppSyncRealTimeProvider – {id: "", observer: null, query: "", …}
{id: "", observer: null, query: "", variables: {}}Object

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

@chrisbonifacio chrisbonifacio self-assigned this Sep 14, 2021
@chrisbonifacio chrisbonifacio added DataStore Related to DataStore category pending-triage Issue is pending triage labels Sep 14, 2021
@iartemiev
Copy link
Member

Hey @applyinnovations, DataStore only invokes the custom error handler when it encounters an AppSync mutation error. That's the current expected behavior. The custom error handler's purpose is to allow you to write business logic for handling various AppSync error scenarios and adjust the locally-persisted records accordingly.

For example:

  1. User creates a new record on a model where they don't have Create permissions
  2. DataStore does not enforce auth rules in the client, so it persists the data locally and sends a mutation request to AppSync
  3. AppSync rejects the mutation
  4. DataStore invokes custom error handler
  5. Error handler business logic tells DataStore to delete the record from the local store

For catching client-side errors - such as field validation - we recommend using try...catch blocks when calling DataStore.save and DataStore.delete.

@iartemiev iartemiev added pending-response and removed pending-triage Issue is pending triage labels Sep 14, 2021
@applyinnovations
Copy link
Author

Hey @iartemiev, thanks for the clarification. When performing a mutation with missing fields, DataStore still creates the data locally, however does not throw the custom error handler. Wouldnt this be a prime example of when to use the error handler to remove the locally created data?

@iartemiev
Copy link
Member

Are you seeing a network request error in that case?

@applyinnovations
Copy link
Author

applyinnovations commented Sep 15, 2021

@iartemiev There is no network error, however the network response contains a graphql error.

Request Body:

query: "mutation operation($input: CreateCategoryInput!) {\n  createCategory(input: $input) {\n    id\n    name\n    color\n    createdAt\n    updatedAt\n    _version\n    _lastChangedAt\n    _deleted\n  }\n}\n"
variables: {input: {id: "6d55ca74-92dc-4a44-9248-9d3e9d43597e"}}
input: {id: "6d55ca74-92dc-4a44-9248-9d3e9d43597e"}
id: "6d55ca74-92dc-4a44-9248-9d3e9d43597e"

Response (Status code 200) Body:

{
   "data":null,
   "errors":[
      {
         "path":null,
         "locations":[
            {
               "line":1,
               "column":20,
               "sourceName":null
            }
         ],
         "message":"Variable 'input' has coerced Null value for NonNull type 'String!'"
      }
   ]
}

Debug logs:

[Log] [DEBUG] 01:17.731 RestClient - POST – "https://sn4fnspmfbdm5dwl5okhzqigce.appsync-api.ap-southeast-2.amazonaws.com/graphql"
[Warning] [WARN] 01:17.879 DataStore - failed to execute errorHandler – Error: Field name is required
Error: Field name is required
[Log] [DEBUG] 01:17.880 DataStore - Mutation sent successfully with authMode: AMAZON_COGNITO_USER_POOLS

cdae68db-c054-4f08-9850-bb48a9c41dad

As you can see in the screenshot, the local state contains the data, however the graphql server rejected the change but because our custom errorHandler provided in DataStore.configure is not used, we cannot fix the local state.

@nubpro
Copy link
Contributor

nubpro commented Sep 15, 2021

Similar to #7860

@applyinnovations
Copy link
Author

applyinnovations commented Sep 20, 2021

On a similar note, how are we expected to handle authorisation errors? They do not throw the default handler or custom handler.

{
   "data":{
      "deletePlaceBenefit":null
   },
   "errors":[
      {
         "path":[
            "deletePlaceBenefit"
         ],
         "data":null,
         "errorType":"Unauthorized",
         "errorInfo":null,
         "locations":[
            {
               "line":2,
               "column":3,
               "sourceName":null
            }
         ],
         "message":"Not Authorized to access deletePlaceBenefit on type PlaceBenefit"
      }
   ]
}

However DataStore commits the change locally and the user is unaware the mutation failed.

@applyinnovations
Copy link
Author

@iartemiev what is the recommended method of fixing the local DataStore state in both of these scenarios? The response is thrown by the server not the client, wouldn't this be an ideal case to use the handler specified in DataStore.configure?

@chrisbonifacio chrisbonifacio added investigating This issue is being investigated and removed pending-response labels Sep 23, 2021
@chrisbonifacio chrisbonifacio added needs-discussion Used for internal discussions and removed investigating This issue is being investigated labels Sep 30, 2021
@chrisbonifacio chrisbonifacio added feature-request Request a new feature bug Something isn't working and removed feature-request Request a new feature labels Sep 30, 2021
@chrisbonifacio
Copy link
Member

I was able to reproduce the issue. Although this is currently expected behavior, I'm going to label this as a feature request that needs discussion because I do see the inconvenience in having to validate the result of the DataStore.save operation in order to check whether or not the GraphQL mutation was successful.

@chrisbonifacio chrisbonifacio added feature-request Request a new feature and removed bug Something isn't working labels Sep 30, 2021
@applyinnovations
Copy link
Author

Thank you @chrisbonifacio for following up on this. I believe using the error handler to repair local state is an essential feature, however it is useless unless it covers all mutation events!

@cfbo
Copy link

cfbo commented Oct 13, 2021

We also have this issue. I can reproduce it for authorization errors (similar to that #8891 (comment)).
From what I understood there isn't any workaround at the moment, right?

Also, is there a default error handler in the Datastore? If I add the code below, am I overriding a default handler or just adding one? What I mean is, if I add that code it shouldn't cause any issue right?

DataStore.configure({
  errorHandler: (error) => {
    console.log(error);
  },
});

@applyinnovations
Copy link
Author

@chrisbonifacio could you please give an update on this? We cannot release our new version to production until this issue is resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DataStore Related to DataStore category feature-request Request a new feature needs-discussion Used for internal discussions
Projects
None yet
Development

No branches or pull requests

4 participants