-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(browser): allow sentry in safari extension background page #13209
feat(browser): allow sentry in safari extension background page #13209
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @undead-voron thanks for opening this PR! I think this sounds reasonable given we've done this before and it only applies to extensions running in dedicated pages, not within a host page. Would you mind adding a test for it? A simple one here is fine for me:
sentry-javascript/packages/browser/test/sdk.test.ts
Lines 202 to 223 in 0ca8821
it.each(['chrome-extension', 'moz-extension', 'ms-browser-extension'])( | |
"doesn't log a browser extension error if executed inside an extension running in a dedicated page (%s)", | |
extensionProtocol => { | |
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); | |
// @ts-expect-error - this is a hack to simulate a dedicated page in a browser extension | |
delete WINDOW.location; | |
// @ts-expect-error - this is a hack to simulate a dedicated page in a browser extension | |
WINDOW.location = { | |
href: `${extensionProtocol}://mock-extension-id/dedicated-page.html`, | |
}; | |
Object.defineProperty(WINDOW, 'browser', { value: { runtime: { id: 'mock-extension-id' } }, writable: true }); | |
init(options); | |
expect(consoleErrorSpy).toBeCalledTimes(0); | |
consoleErrorSpy.mockRestore(); | |
WINDOW.location = originalLocation; | |
}, | |
); |
heads-up: I'm only assigning myself to track internally that I'm reviewing it. I'm not gonna take over for the test, please continue :) |
Hi @Lms24 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for contributing!
This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #13209 --- Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
Add
safari-web-extension:
to the list of allowed extension protocols. This chage should allow sentry/browser to work in safari browser extensions.