Add try-catch around window.sessionStorage access

[chenba]

Pull Request checklist

• Quality: Make sure this PR builds and runs cleanly.
  • Inside the glean/ folder, run:
    • npm run test Runs all tests
    • npm run lint Runs all linters
• Tests: This PR includes thorough tests or an explanation of why it does not
  • I was not able to find any existing tests around sessionStorage or some kind of mock for the window object
• Changelog: This PR includes a changelog entry to CHANGELOG.md or an explanation of why it does not need one
  • I'm not sure if we are going to tag a release right away. I can update the changelog if that's the case.
• Documentation: This PR includes documentation changes, an explanation of why it does not need that or a follow-up bug has been filed to do that work
  • This PR doesn't change the API. It prevents an uncaught error that could break some JS based webapps when Glean is in the dependency tree.

[Dexterp37]

Instead of wrapping the whole block in try-catch, what about something like:

let hasStorage = false;
try {
    hasStorage = typeof window !== "undefined"
        && typeof window.sessionStorage !== "undefined";
} catch (e) {
    console.error("No session storage available", e)
}

if (hasStorage) {
// ...

This would not increase indentation and would reduce the scope of the try block.

[chenba]

Updated the patch based on @Dexterp37's suggestion. Thank you, @Dexterp37

[Dexterp37]

@chenba do you need a release cut ? Is this blocking something or was this problem found by some other investigation?

[chenba]

@chenba do you need a release cut ? Is this blocking something or was this problem found by some other investigation?

I do need a release. That error is blocking https://accounts.firefox.com/cookies_disabled from being rendered if the user disabled cookies/session storage/local storage in their browser preferences.

[Dexterp37]

I do need a release. That error is blocking https://accounts.firefox.com/cookies_disabled from being rendered if the user disabled cookies/session storage/local storage in their browser preferences.

Got it. @chenba would you kindly add a changelog entry ? I'll make sure to cut a release tomorrow morning.

[chenba]

Updated patch to include an entry in the changelog under "Unreleased changes".

[Dexterp37]

Updated patch to include an entry in the changelog under "Unreleased changes".

Thanks. I merged this, but now I'm also wondering if this would lead to other problems if the generated metrics are ever called (or if windows.Glean.* stuff) is ever called. @chenba I'm ok with cutting a release for this, but have you tested it? Does it solve your problem or do we need more changes?

[chenba]

Thanks, @Dexterp37.

have you tested it? Does it solve your problem or do we need more changes?

I did test it. And it solves our issue, which is that the error is thrown at import.

this would lead to other problems if the generated metrics are ever called (or if windows.Glean.* stuff) is ever called.

I'm not familiar with this code base but it looks like the window.Glean API is mainly for development and debugging? If there's an issue then it'd be developer facing issue and not an end user one perhaps. (Not that accidentally leaving debugging code in production is unheard of.)

[Dexterp37]

Thanks, @Dexterp37.

have you tested it? Does it solve your problem or do we need more changes?

I did test it. And it solves our issue, which is that the error is thrown at import.

Ok, cutting a release