chore(auth): debounce refreshAuthTokens #12845

israx · 2024-01-12T17:09:58Z

Description of changes

The purpose of this PR is to debounce sync concurrent calls to the fetchAuthSession API. This approach is similar to the current debouncing logic implemented in v5

Issue #, if available

Description of how you validated changes

Checklist

PR description included
yarn test passes
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.

AllanZhengYP

Good to see timer is removed. Thank you!

HuiSF

Ty for improving the impl.

packages/core/src/utils/deDupCallback.ts

HuiSF · 2024-01-12T20:53:23Z

packages/datastore/src/sync/processors/subscription.ts

@@ -279,7 +279,7 @@ class SubscriptionProcessor {

 				try {
 					// retrieving current token info from Cognito UserPools
-					const session = await await fetchAuthSession();


packages/core/__tests__/utils/debounceCallback.test.ts

HuiSF · 2024-01-12T20:57:50Z

packages/core/__tests__/utils/debounceCallback.test.ts

+	it('should allow to invoke the callback again after the promise has being resolved', async () => {
+		const debouncedCallback = debounceCallback(mockServiceCallback);
+		for (let i = 0; i < numberOfConcurrentCalls; i++) {
+			debouncedCallback();
+		}
+
+		await debouncedCallback();
+
+		debouncedCallback();
+		expect(mockServiceCallback).toHaveBeenCalledTimes(2);
+	});


This test may not be accurate - the mockServiceCallback called times should increase after the promised returned by it resolves, and when debouncedCallback gets called again, but in this test, the mockServiceCallback doesn't even return a promise.

the mockServiceCallback doesn't return a promise but the deDupe function does because it creates a promise on top of the callback passed. What I'm trying to do is to call the deDuped function concurrently and then calling it again when it was able to resolve.

packages/core/src/utils/deDupCallback.ts

package.json

packages/auth/src/providers/cognito/utils/refreshAuthTokens.ts

packages/core/src/utils/deDupeAsyncRequests.ts

packages/core/__tests__/utils/deDupeAsyncRequests.test.ts

packages/core/src/utils/deDupeAsyncRequests.ts

packages/auth/src/providers/cognito/utils/refreshAuthTokens.ts

packages/core/src/utils/deDupeAsyncRequests.ts

packages/core/src/utils/deDupeAsyncFunction.ts

elorzafe

Looks good,

nit: why not have a unit for refreshAuthTokens calls are deduped?

This reverts commit 5fc446e.

* Revert "chore(auth): debounce refreshAuthTokens (#12845)" This reverts commit 5fc446e. * chore: update workflow

* chore(auth): debounce refreshAuthTokens (#12845) * chore: add debounce callback helper * chore: add unit tests * chore: debounce fetchAuthSession * chore: fix unit test * chore: fix bundle size limits * chore: update debounce logic * chore: update dedup logic * chore: debounce refreshAuthTokens * chore: fix bundle size * chore: address feedback * chore: fix unit test * chore: address feedback * chore: update yarn.lock * chore: address feedbak * fix(data): update DataStore to send correct Control Messages when starting (#12861) * chore(release): Publish [ci skip] - @aws-amplify/adapter-nextjs@1.0.13 - @aws-amplify/analytics@7.0.13 - @aws-amplify/api@6.0.13 - @aws-amplify/api-graphql@4.0.13 - @aws-amplify/api-rest@4.0.13 - @aws-amplify/auth@6.0.13 - aws-amplify@6.0.13 - @aws-amplify/core@6.0.13 - @aws-amplify/datastore@5.0.13 - @aws-amplify/datastore-storage-adapter@2.1.13 - @aws-amplify/geo@3.0.13 - @aws-amplify/interactions@6.0.13 - @aws-amplify/notifications@2.0.13 - @aws-amplify/predictions@6.0.13 - @aws-amplify/pubsub@6.0.13 - @aws-amplify/react-native@1.0.13 - @aws-amplify/react-native-example@0.0.14 - @aws-amplify/rtn-push-notification@1.2.13 - @aws-amplify/rtn-web-browser@1.0.13 - @aws-amplify/storage@6.0.13 - tsc-compliance-test@0.1.13 * chore(release): update API docs [ci skip] * chore: enable codecov (#12876) * docs(auth,analytics): adding in-line docs for public apis (#12882) * docs(auth,analytics): adding in-line docs for public apis * fix: unblock the max length lint * chore(react-native): use react-native 0.71.0 as the dev dep * fix: Default branch resolution when running E2E tests (#12910) * fix: Lambda auth config value (#12922) * chore: renable caching the package list in GH actions * chore: remove unused codecov package (has been deprecated) * feat(api): expose HTTP response from API errors (#12835) --------- Co-authored-by: Hui Zhao <10602282+HuiSF@users.noreply.github.com> Co-authored-by: Jim Blanchard <jim.l.blanchard@gmail.com> * chore: temporarily disable codecov GH action integration (#12928) * ci: run flaky data e2es without retry (#12758) * ci: run flaky data e2es without retry * only run on chrome * restore workflow --------- Co-authored-by: Aaron S <94858815+stocaaro@users.noreply.github.com> * fix(core): Amplify.configure dispatches Hub event with unparsed config object (#12930) * chore(repo): use typescript 5.0.2 across workspace * chore(repo): refactor tsconfig hierarchy - move the base tsconfig into the root of the workspace - unifying tsconfig tsconfig.build tsconfig.test and tsconfig.watch settings * chore(repo): promote rollup dependencies into workspace - remove unused build.js script * chore(repo): add build:watch using rollup * chore(repo): setup eslint * chore(adapter-nextjs): migrate to eslint * fix(repo): test-github-actions using js-yaml already removed API * chore(repo): add Code Spell Checker to recommended extension list * chore(repo): add formatOn actions to formattingToggle controlables * fix: not appending notification configs * chore(release): Publish [ci skip] - @aws-amplify/adapter-nextjs@1.0.14 - @aws-amplify/analytics@7.0.14 - @aws-amplify/api@6.0.14 - @aws-amplify/api-graphql@4.0.14 - @aws-amplify/api-rest@4.0.14 - @aws-amplify/auth@6.0.14 - aws-amplify@6.0.14 - @aws-amplify/core@6.0.14 - @aws-amplify/datastore@5.0.14 - @aws-amplify/datastore-storage-adapter@2.1.14 - @aws-amplify/geo@3.0.14 - @aws-amplify/interactions@6.0.14 - @aws-amplify/notifications@2.0.14 - @aws-amplify/predictions@6.0.14 - @aws-amplify/pubsub@6.0.14 - @aws-amplify/react-native@1.0.14 - @aws-amplify/react-native-example@0.0.15 - @aws-amplify/rtn-push-notification@1.2.14 - @aws-amplify/rtn-web-browser@1.0.14 - @aws-amplify/storage@6.0.14 - tsc-compliance-test@0.1.14 * Revert "chore(repo): use typescript 5.0.2 across workspace" (#12941) * Revert "chore(repo): use typescript 5.0.2 across workspace" This reverts commit e20782e. * chore(api-graphql): temporarily install rollup to package * fix(repo): rollup generated sourcemap has wrong src path (#12947) * fix(datastore): Treat head as optional to avoid errors (#12936) * fix(datastore): Treat head as optional to avoid errors * Add testing and fix follow-on issue * fix(api-graphql): wrong arguments for GET operation of a CPK model * fix: Remove special e2e treatment for validated tests (#12946) fix: Remove special integ treament for validated tests * fix(datastore): Limit docs integ test to one browser to improve stability (#12937) * fix(api-graphql): same results returned for queries on the same model with different selection set - caused by the incomplete GraphQL documents caching * Update branch to work with merged updates from main --------- Co-authored-by: israx <70438514+israx@users.noreply.github.com> Co-authored-by: David McAfee <mcafd@amazon.com> Co-authored-by: Jim Blanchard <jim.l.blanchard@gmail.com> Co-authored-by: aws-amplify-bot <aws@amazon.com> Co-authored-by: Hui Zhao <10602282+HuiSF@users.noreply.github.com> Co-authored-by: AllanZhengYP <zheallan@amazon.com> Co-authored-by: Hui Zhao <zhohz@amazon.com> Co-authored-by: Ivan Artemiev <29709626+iartemiev@users.noreply.github.com> Co-authored-by: Aaron S <94858815+stocaaro@users.noreply.github.com> Co-authored-by: ManojNB <manojnb95@gmail.com> Co-authored-by: ManojNB <manojnb@amazon.com> Co-authored-by: Francisco Rodriguez <frodriguez.cs@gmail.com>

israx added 5 commits January 12, 2024 12:06

chore: add debounce callback helper

4b27260

chore: add unit tests

751b15b

chore: debounce fetchAuthSession

80c2b60

chore: fix unit test

4ab0de2

chore: fix bundle size limits

b0d982a

israx requested review from a team as code owners January 12, 2024 17:09

chore: update debounce logic

3419b8f

AllanZhengYP previously approved these changes Jan 12, 2024

View reviewed changes

israx added 3 commits January 12, 2024 14:45

chore: update dedup logic

4bbeba0

chore: debounce refreshAuthTokens

e3df8aa

chore: fix bundle size

0db3893

israx dismissed AllanZhengYP’s stale review via 0db3893 January 12, 2024 20:33

israx changed the title ~~chore(auth): debounce fetchAuthSession API~~ chore(auth): debounce refreshAuthTokens Jan 12, 2024

HuiSF reviewed Jan 12, 2024

View reviewed changes

HuiSF reviewed Jan 13, 2024

View reviewed changes

packages/core/src/utils/deDupCallback.ts Outdated Show resolved Hide resolved

israx added 2 commits January 16, 2024 09:58

chore: address feedback

a717e20

chore: fix unit test

b46967d

cshfang previously approved these changes Jan 16, 2024

View reviewed changes

HuiSF reviewed Jan 16, 2024

View reviewed changes

packages/core/__tests__/utils/deDupeAsyncRequests.test.ts Outdated Show resolved Hide resolved

packages/core/__tests__/utils/deDupeAsyncRequests.test.ts Outdated Show resolved Hide resolved

packages/core/__tests__/utils/deDupeAsyncRequests.test.ts Outdated Show resolved Hide resolved

cshfang reviewed Jan 16, 2024

View reviewed changes

packages/core/src/utils/deDupeAsyncRequests.ts Outdated Show resolved Hide resolved

packages/auth/src/providers/cognito/utils/refreshAuthTokens.ts Outdated Show resolved Hide resolved

HuiSF reviewed Jan 16, 2024

View reviewed changes

packages/core/src/utils/deDupeAsyncRequests.ts Outdated Show resolved Hide resolved

chore: address feedback

4ce72c3

israx dismissed cshfang’s stale review via 4ce72c3 January 17, 2024 04:04

AllanZhengYP previously approved these changes Jan 17, 2024

View reviewed changes

chore: update yarn.lock

1444ffd

israx dismissed AllanZhengYP’s stale review via 1444ffd January 17, 2024 17:46

cshfang reviewed Jan 17, 2024

View reviewed changes

packages/core/src/utils/deDupeAsyncFunction.ts Outdated Show resolved Hide resolved

packages/core/src/utils/deDupeAsyncFunction.ts Outdated Show resolved Hide resolved

packages/core/src/utils/deDupeAsyncFunction.ts Outdated Show resolved Hide resolved

chore: address feedbak

a976d76

cshfang approved these changes Jan 17, 2024

View reviewed changes

elorzafe approved these changes Jan 17, 2024

View reviewed changes

iartemiev approved these changes Jan 18, 2024

View reviewed changes

israx merged commit 5fc446e into aws-amplify:main Jan 18, 2024
30 checks passed

david-mcafee added a commit that referenced this pull request Jan 18, 2024

Revert "chore(auth): debounce refreshAuthTokens (#12845)"

182b97a

This reverts commit 5fc446e.

david-mcafee added a commit to david-mcafee/amplify-js that referenced this pull request Jan 18, 2024

Revert "chore(auth): debounce refreshAuthTokens (aws-amplify#12845)"

14b5322

This reverts commit 5fc446e.

israx added a commit to israx/amplify-js that referenced this pull request Jan 18, 2024

Revert "chore(auth): debounce refreshAuthTokens (aws-amplify#12845)"

fbfa1f1

This reverts commit 5fc446e.

israx added a commit that referenced this pull request Jan 18, 2024

Revert "chore(auth): debounce refreshAuthTokens (#12845)" (#12862)

f774256

This reverts commit 5fc446e.

israx added a commit that referenced this pull request Jan 19, 2024

Revert/fetch auth session dedupe (#12863)

01b0873

* Revert "chore(auth): debounce refreshAuthTokens (#12845)" This reverts commit 5fc446e. * chore: update workflow

ashwinkumar6 mentioned this pull request Feb 7, 2024

feat(logging): CW client, deviceId & NetworkMonitor #12907

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(auth): debounce refreshAuthTokens #12845

chore(auth): debounce refreshAuthTokens #12845

israx commented Jan 12, 2024 •

edited

AllanZhengYP left a comment

HuiSF left a comment

HuiSF Jan 12, 2024

HuiSF Jan 12, 2024

israx Jan 16, 2024

elorzafe left a comment

chore(auth): debounce refreshAuthTokens #12845

chore(auth): debounce refreshAuthTokens #12845

Conversation

israx commented Jan 12, 2024 • edited

Description of changes

Issue #, if available

Description of how you validated changes

Checklist

AllanZhengYP left a comment

Choose a reason for hiding this comment

HuiSF left a comment

Choose a reason for hiding this comment

HuiSF Jan 12, 2024

Choose a reason for hiding this comment

HuiSF Jan 12, 2024

Choose a reason for hiding this comment

israx Jan 16, 2024

Choose a reason for hiding this comment

elorzafe left a comment

Choose a reason for hiding this comment

israx commented Jan 12, 2024 •

edited