Skip to content
New issue

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

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

Already on GitHub? # to your account

fix(core): Check for storage initialization errors #13938

Merged
merged 7 commits into from
Nov 8, 2024

Conversation

cwomack
Copy link
Member

@cwomack cwomack commented Oct 18, 2024

Description of changes

Added Session Storage and Default Storage try catch blocks

Issue #, if available

Resolves #13747

Description of how you validated changes

Checklist

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

Checklist for repo maintainers

  • Verify E2E tests for existing workflows are working as expected or add E2E tests for newly added workflows
  • New source file paths included in this PR have been added to CODEOWNERS, if appropriate

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

@cwomack cwomack requested review from haverchuck, cshfang, jimblanc, HuiSF and a team as code owners October 18, 2024 18:55
@cwomack cwomack requested a review from HuiSF October 18, 2024 21:28
HuiSF
HuiSF previously approved these changes Oct 18, 2024
Copy link
Contributor

@jimblanc jimblanc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! Looks like we need to tweak some bundle size tests

@@ -5,7 +5,6 @@
/* eslint-disable camelcase */

/* Does not like exhaustive checks */
/* eslint-disable no-case-declarations */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was from running yarn lint:fix on core

ashika112
ashika112 previously approved these changes Oct 31, 2024
@cwomack cwomack requested a review from ashika112 October 31, 2024 20:15
ashika112
ashika112 previously approved these changes Oct 31, 2024
Co-authored-by: AllanZhengYP <zheallan@amazon.com>
@cwomack cwomack requested a review from AllanZhengYP November 8, 2024 04:48
? window.localStorage
: new InMemoryStorage();

const logger = new ConsoleLogger('CoreStorageUtils');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont' think we have a convention for logger name but this looks good to me!

Copy link
Member

@AllanZhengYP AllanZhengYP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! TY @cwomack

@cwomack cwomack merged commit 0f50917 into aws-amplify:main Nov 8, 2024
30 checks passed
yuhengshs added a commit that referenced this pull request Nov 13, 2024
@cwomack cwomack deleted the fix/storage-init-errors branch November 25, 2024 18:13
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Client side storage access is not wrapped in try catch block
6 participants