fix: Check for undefined faro instancein isWebStorageAvailable catch block #757
Conversation
|
|
edkimmel
changed the title
edkimmel
changed the title
There was a problem hiding this comment.
Hey @edkimmel, thank you so much for your interest and support in the project, as well as the time you allocated to work on this. I really appreciate it, although this is a small change, would you be able to provide a screenshot or a log, or ideally a test case that this might fail? From my understanding, the faro instance will be instantatiated here.
To be clear, I am more than happy to merge this, but I am only marking it as 'Request changes' to make sure we resolve the conversation
| @@ -25,7 +25,8 @@ export function isWebStorageAvailable(type: StorageMechanism): boolean { | |||
| return true; | |||
| } catch (error) { | |||
| // the above can throw | |||
| faro.internalLogger?.info(`Web storage of type ${type} is not available. Reason: ${error}`); | |||
| // this is called during module init, when the global instance may not be available | |||
There was a problem hiding this comment.
This is not necessarily called during global module init if you would be able to rephrase the comment for clarity.
|
The logs we are seeing on our end are I have not been able to reproduce locally. Scanning through faro, I believe this is the only callsite that could produce that error. The other calls to faro.internalLogger all check some other property of the global faro instance (faro.config) first, which would be the line thrown from instead. |
Hey once again. I was on PTO last week so apologies for catching up with your replies a week later. Do you have any other stack traces that might be helpful? Although I am leaning towards your original change being the solution. I just want to get more context so that we can have precedence and ideally create a test for it |
|
Hi @edkimmel want to follow up on this. Do you still run into the issue? |
Why
If storage throws an exception during the module init calls to isWebStorageAvailable, the global faro instance is not set and this crashes.
What
Optional chain the faro instance itself in this catch block.
Links
Checklist