fix(@aws-amplify/datastore): Retry mutation if GraphQL timeout due to bad network condition.

[nubpro]

Issue #, if available:
I didn't open a new issue, but you can reproduce the problem as follow:

1. Open a react native app on iOS.
2. Go to device's Settings > Developer > Network Link Conditioner > Enable and select 100% Loss.
3. Immediately switch back to the app, update a record.
4. Approximately 15 seconds later, you'll see a warning appearing on your app. (you can refer to the attachment below)
   

The consequences of not handling this error is that DataStore will dequeue this mutation and the record will never be updated to the server which is a big nono.

Description of changes:
This PR will make sure mutation processor to perform retry if the mutation GraphQL returns timeout of 0ms exceeded.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[codecov]

Codecov Report

Merging #6542 (b9ab211) into main (99d3c55) will increase coverage by 0.58%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #6542      +/-   ##
==========================================
+ Coverage   74.80%   75.38%   +0.58%     
==========================================
  Files         215      215              
  Lines       13522    13524       +2     
  Branches     2661     2663       +2     
==========================================
+ Hits        10115    10195      +80     
+ Misses       3208     3127      -81     
- Partials      199      202       +3     

Impacted Files	Coverage Δ
packages/api-graphql/src/GraphQLAPI.ts	85.16% <ø> (ø)
packages/datastore/src/sync/processors/mutation.ts	63.08% <100.00%> (+50.16%)	⬆️
packages/datastore/src/types.ts	81.08% <0.00%> (+4.05%)	⬆️
...ore/src/storage/adapter/getDefaultAdapter/index.ts	100.00% <0.00%> (+25.00%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99d3c55...b9ab211. Read the comment docs.

[ericclemmons]

This looks related to our underlying axios implementation:

axios/axios#2103 (comment)

@manueliglesias Is this a resilient enough message to key off of?

[manueliglesias]

Good catch @ericclemmons, I don't think it would be resilient enough, I think it would be better to include additional keys in the GraphQL error if needed on GraphQLAPI.ts (e.g. an error code with values like the ECONNABORTED mentioned in the linked issue, or similar)

amplify-js/packages/api-graphql/src/GraphQLAPI.ts

Line 291 in c135284

errors: [new GraphQLError(err.message)],

[nubpro]

Hey thanks for the review! Would you guys do the honor to make the changes so they would work gracefully.

I'll be happy to test it out on my end!

[nubpro]

May I know how can I pass the errorCode into GraphQLError interface?

I was thinking of passing the original error, is that ideal?

			response = {
				data: {},
				errors: [new GraphQLError(err.message, null, null, null, null, err)],
			};

[iartemiev]

I was able to successfully test the PR in iOS simulator

1. Reproduced the issue by following the steps in the description with the current latest version of aws-amplify
2. Built and published the changes in the PR to my locally-running Verdaccio registry
3. Installed the aws-amplify dependencies into the sample app from Verdaccio
4. Confirmed that the changes in the PR resolve the issues, namely:
   a. Network Error with code ECONNABORTED gets handled
   b. Reachability is set to false
   c. After Network Link Conditioner is disabled/network condition is stable, Reachability is set to true and the mutation is retried and the request succeeds
   d. Subsequent mutations succeed as well

Since it's not possible to simulate 100% Packet Loss (or other network conditions) in Detox (the e2e testing framework we use for RN apps) we won't be able to add an e2e test for this fix but we should be able to at least add a unit test that captures this. I will work on adding the test today and will merge the PR afterward.

Thank you so much for this contribution, @nubpro!

[iartemiev]

Unit test has been added: https://github.com/aws-amplify/amplify-js/pull/6542/files#diff-b21a0f0d7db920e7ce7ad487d762f40fc3aca6b288e621a8db17347f50a480db

Going to ask @amhinson to give the test a once-over and then we'll merge this PR today.

[iartemiev]

Approving. See #6542 (comment) for more detail

[amhinson]

LGTM 👍

[github-actions]

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.