Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

2924: Catch SecurityError when accessing localStorage #2974

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

steffenkleinle
Copy link
Member

Short description

Catch SecurityError when accessing localStorage and prevent app from crashing.

Proposed changes

  • Wrap access of localStorage in a try/catch block

Side effects

None.

Testing

I did not find a way to test this locally (as in: using a development server on localhost) due to me being unable to block localhost from accessing the local storage. Let me know if you can think of another way.

Resolved issues

Fixes: #2924


Copy link

codeclimate bot commented Oct 25, 2024

Code Climate has analyzed commit bcf9dda and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 80.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 74.1%.

View more on Code Climate.

Copy link
Contributor

@LeandraH LeandraH left a comment

Choose a reason for hiding this comment

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

Nice! I can't think of another way either but the test seems pretty trustworthy to me.

Comment on lines +24 to +27
// Try to prevent the following error crashing the app:
// SecurityError: Failed to read the 'localStorage' property from 'Window': Access is denied for this document.
// See #2924
reportError(e)
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 If we want to leave this comment in permanently, then I think we should rephrase it. If not, I guess we should have some kind of reminder to check in a couple of weeks or months if this issue still occurs. Any thoughts?

Comment on lines +37 to +40
// Try to prevent the following error crashing the app:
// SecurityError: Failed to read the 'localStorage' property from 'Window': Access is denied for this document.
// See #2924
reportError(e)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App crashing if local storage access is denied
2 participants