feat(wasm): initialised sentryWasmImages for webworkers #18812
Conversation
|
Thanks for the contribution @harshit078. Are you done working on this or are you done (asking since it's a draft). |
|
Hey @andreiborza , yes I have finished working on this PR and converted it to review. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
working on comments addressed by cursor ! |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
dev-packages/browser-integration-tests/suites/wasm/webWorker/test.ts
Outdated
Show resolved
Hide resolved
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
dev-packages/browser-integration-tests/suites/wasm/webWorker/assets/worker.js
Fixed
Show fixed
Hide fixed
| } | ||
|
|
||
| try { | ||
| const { instance } = await WebAssembly.instantiateStreaming(fetch(wasmUrl), { |
Check warning
Code scanning / CodeQL
Client-side request forgery Medium
|
Hey @andreiborza , the PR is ready to review. |
|
@harshit078 awesome, thanks! I'll take a look. |
| } | ||
|
|
||
| // Handle messages from the main thread | ||
| self.addEventListener('message', async event => { |
Check warning
Code scanning / CodeQL
Missing origin verification in `postMessage` handler Medium
| /** | ||
| * Registers a WASM module and forwards its debug image to the parent thread. | ||
| */ | ||
| function registerModuleAndForward( |
There was a problem hiding this comment.
m: I think we should call registerModule form registry.ts in here, that way we get some deduplication going when needed. It'll also get rid of this almost identical code duplication.
We'll need to adapt registerModule to return the image so we can post it in this method.
| @@ -0,0 +1,86 @@ | |||
| // This worker manually just replicates what the actual Sentry.registerWebWorkerWasm() does | |||
There was a problem hiding this comment.
m: Could we highlight in the comment why we are doing that as opposed to actually using Sentry. registerWebWorkerWasm() please?
|
@harshit078 thanks again. Just had a look, looks mostly fine to me. Just a minor comment around code duplication. |
2 participants
Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint) & (yarn test).Closes #18779