fix(@aws-amplify/datastore): introduce "settlement" guarantees to stop() and clear()

[svidgen]

Description of changes

Prior to this PR, DataStore.clear() and DataStore.stop did not provide guarantees that background work was complete before returning. This caused leakage between DataStore execution contexts (e.g., stopping and clearing DataStore to update selective sync) which resulted in rogue errors and potential data corruption.

This leakage also became very evident during certain unit/interaction test cases, wherein it manifest as unstable tests.

This PR introduces a BackgroundProcessManager which is can be used to track lingering promises, timeouts, subscriptions, and "cleaner" functions. It provides termination signals for BG procs to listen to and terminate early if possible when a BackgroundProcessManager requests it. Instances of BackgroundProcessManager have been added to all core DataStore processes and promises, and tests have been added which test all paths — with a partial exception for subscriptions, which require more test harnessing to fully test (to be added with bug fixes in the coming weeks).

Reviewer guidance

1. Turn off whitespace changes. ~~2. Forgive the cruft ... Working on it!~~

Issue #, if available

Description of how you validated changes

1. Added a lot of tests.
2. Yarn linked to app and ran DataStore operations, including observes, clears, etc..

TODO:

• [x] fix Auth TS check
• [x] clean up bad pauses in tests
• [ ] ~propagate termination to API.graphql() calls~ (Deferring to a subsequent sprint/PR, as will require more complex test harnessing)
• [x] link and integ test
• [x] disabled and/or remove console logs, debugs, warns, etc. used for troubleshooting (turn prod ones into logger.debugs)
• [x] rename this.context -> this.pendingWork or this.pendingJobs or something more descriptive
• [x] rename or alias exit() to complete() or something more jobs/process poolish?
• [x] add an isComplete and use in place of onTerminate setting local flags where possible.

Checklist

• [x] PR description included
• [x] yarn test passes
• [x] Tests are changed or added
• [ ] Relevant documentation is changed or added (and PR referenced)

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

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging 980806979015399f30b98f9c305afdde0d7e162b into 870ec87a6b5d6f3aa3a85551faac87a208c354a2 - view on LGTM.com

new alerts:

• 2 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging 51aee7cc8047f993dadf16e82bc86cd601bb0ea7 into 870ec87a6b5d6f3aa3a85551faac87a208c354a2 - view on LGTM.com

new alerts:

• 2 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging 0877330f1a14aa648583aa501344cef6c022b2b6 into 870ec87a6b5d6f3aa3a85551faac87a208c354a2 - view on LGTM.com

new alerts:

• 2 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging bb021d660f3c6efbbd0bac1ce7da5acb3d429bbc into 870ec87a6b5d6f3aa3a85551faac87a208c354a2 - view on LGTM.com

new alerts:

• 2 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging f447ac635b481c83ad9d7ed55e05c72f564741ed into de0441b4fa67409ccbc630c42890e2c58ee779fb - view on LGTM.com

new alerts:

• 2 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging 2a637ffd0c994c327ac8e1bef1448824de70b28e into f6e2ca25785bb30ab6040f1f8c163e6069ccc392 - view on LGTM.com

new alerts:

• 2 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 3 alerts when merging a755e6a8733ff4dd481237dcba2144e8f2b39512 into f6e2ca25785bb30ab6040f1f8c163e6069ccc392 - view on LGTM.com

new alerts:

• 3 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 4 alerts when merging 7b237a733c2cf098100fe495b414d6cc729ad66a into 8e49b0d53bc7a8c373289cf5aa2172284c76ecd6 - view on LGTM.com

new alerts:

• 4 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 3 alerts when merging d379ffce4ae8c6b967f099f6b0ec3d135b99d0d8 into 8e49b0d53bc7a8c373289cf5aa2172284c76ecd6 - view on LGTM.com

new alerts:

• 3 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 3 alerts when merging f9d4739b8f627bea35233c336759444f0139a082 into 8e49b0d53bc7a8c373289cf5aa2172284c76ecd6 - view on LGTM.com

new alerts:

• 3 for Unused variable, import, function or class

[codecov-commenter]

Codecov Report

Merging #10055 (6cf87b0) into main (bcf4498) will increase coverage by 0.12%. The diff coverage is 80.92%.

@@            Coverage Diff             @@
##             main   #10055      +/-   ##
==========================================
+ Coverage   84.08%   84.20%   +0.12%     
==========================================
  Files         259      260       +1     
  Lines       19137    19393     +256     
  Branches     4105     4153      +48     
==========================================
+ Hits        16091    16330     +239     
- Misses       2957     2974      +17     
  Partials       89       89              

Impacted Files	Coverage Δ
packages/core/src/Credentials.ts	34.67% <0.00%> (ø)
...astore/src/authModeStrategies/multiAuthStrategy.ts	92.75% <ø> (ø)
packages/datastore/src/sync/merger.ts	92.00% <ø> (ø)
packages/datastore/src/util.ts	92.25% <ø> (ø)
packages/datastore/src/sync/index.ts	74.92% <63.41%> (+1.82%)	:arrow_up:
packages/datastore/__tests__/helpers.ts	80.69% <70.51%> (-6.41%)	:arrow_down:
...ages/datastore/src/sync/processors/subscription.ts	74.09% <73.33%> (+0.50%)	:arrow_up:
packages/datastore/src/sync/processors/mutation.ts	74.05% <81.66%> (+1.23%)	:arrow_up:
packages/datastore/src/datastore/datastore.ts	84.35% <91.48%> (+1.04%)	:arrow_up:
packages/datastore/src/sync/processors/sync.ts	88.75% <92.10%> (-0.49%)	:arrow_down:
... and 16 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[lgtm-com[bot]]

This pull request introduces 1 alert when merging f8e07d2b4c2ae3af880a88ec204846eddc0458f5 into 5d50d43fe37cc8046fbb69180ba31e527993ee84 - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 4e800fa558ad0044c832b1305c5a229a25db5d35 into e6bcf1cd8128b132b9bc06f98bb6694cf23e41b2 - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[danrivett]

Curious @stocaaro if this PR has been replaced with a different PR to address this issue?

I haven't read through the PR commits, but from the description it's definitely something we're looking forward to seeing as we've currenly had to implement custom logic monitoring the outbox queue to try and avoid these sorts of issues and would love that concern to be handled by DataStore instead.

[alharris-at]

@danrivett I think this PR was auto-closed when the parent branch was deleted. FYI @svidgen we probably need to rebase this PR against main now, I don't have permissions to reopen on this repo.

[danrivett]

Thanks @alharris-at that makes sense now, thanks for letting me know.

We were just following this PR in particular, so got notified when it got closed.