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

fix(profiling): Guard from throwing if profiler constructor throws #7328

Merged
merged 2 commits into from Mar 4, 2023

Conversation

JonasBa
Copy link
Member

@JonasBa JonasBa commented Mar 3, 2023

If document-policy header is not set, the profiling API is still present but attempting to initialize the profiler will throw.

This PR guards from that by adding a try/catch block around the profiler constructor.

@JonasBa JonasBa requested a review from lforst March 3, 2023 14:24
@JonasBa JonasBa force-pushed the jb/profiling/feature-check-wrap branch from f902c1d to 4d4b39c Compare March 3, 2023 14:25
@lforst lforst changed the title fix(profiling): guard from throwing if profiler constructor throws fix(profiling): Guard from throwing if profiler constructor throws Mar 3, 2023
Copy link
Member

@lforst lforst left a comment

Choose a reason for hiding this comment

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

lgtm


it('does not throw if Profiler is not available', () => {
// @ts-ignore force api to be undefined
global.window.Profiler = undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to reset this like we do with document, window and location?

Copy link
Member Author

Choose a reason for hiding this comment

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

@lforst I think this is already done in beforeEach where we override the global.window object with the new one from the fresh jsdom instance

Copy link
Member

Choose a reason for hiding this comment

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

Ah right. Didn't see that Profiler was on the window object. Sorry about that!

Copy link
Member Author

Choose a reason for hiding this comment

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

All good :D always better to ask then to miss it :D

@JonasBa JonasBa enabled auto-merge (squash) March 3, 2023 19:42
@JonasBa JonasBa merged commit 770ae30 into develop Mar 4, 2023
@JonasBa JonasBa deleted the jb/profiling/feature-check-wrap branch March 4, 2023 21:36
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.

None yet

2 participants