aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.41k stars 2.11k forks source link

DataStore fails to use custom errorHandler in some cases #8891

Open applyinnovations opened 2 years ago

applyinnovations commented 2 years ago

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

iartemiev commented 2 years ago

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.

applyinnovations commented 2 years ago

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 commented 2 years ago

Are you seeing a network request error in that case?

applyinnovations commented 2 years ago

@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 commented 2 years ago

Similar to #7860

applyinnovations commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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.

applyinnovations commented 2 years ago

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 commented 2 years ago

We also have this issue. I can reproduce it for authorization errors (similar to that https://github.com/aws-amplify/amplify-js/issues/8891#issuecomment-922582761). 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 commented 2 years ago

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