feat(core): Add experimental debug ID based source map upload to Rollup and Vite plugins#192
Conversation
lforst
changed the title
AbhiPrasad
left a comment
AbhiPrasad
left a comment
There was a problem hiding this comment.
This needs tests, especially around the use strict logic and determining source map paths.
|
|
||
| // TODO: Find a more elaborate process to generate this. (Maybe with type checking and built-in minification) | ||
| const DEBUG_ID_INJECTOR_SNIPPET = | ||
| ';!function(){try{var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof self?self:{},n=(new Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]="__SENTRY_DEBUG_ID__",e._sentryDebugIdIdentifier="sdbid-__SENTRY_DEBUG_ID__")}catch(e){}}();'; |
There was a problem hiding this comment.
We can build this whenever we need to change it - same approach as we use here: getsentry/sentry#46780. I don't think we need to automate this, we can just manually run terser.
So everytime this snippet changes, we rebuild the minified JS. We can have this in follow up PR.
| /** | ||
| * Deterministically hashes a string and turns the hash into a uuid. | ||
| */ | ||
| export function stringToUUID(str: string): string { |
There was a problem hiding this comment.
l: Can we have a test that enforces this will return a uuid - sanity check mostly that it behaves ok across Node versions.
| * | ||
| * Note: Currently only functional for Vite and Rollup. | ||
| */ | ||
| debugIdUpload?: { |
There was a problem hiding this comment.
m: Can we add the default values for include/ignore?
There was a problem hiding this comment.
I am not sure if a default for include makes sense, since you either define paths or you don't. Ignore makes sense though, so I added it: a7cf4e3
| }); | ||
|
|
||
| await writeSourceFilePromise; | ||
| await writeSourceMapFilePromise; |
There was a problem hiding this comment.
l: Why double await here? If we want it to run at the same time, can we just do Promise.allSettled?
There was a problem hiding this comment.
I think we want to throw here in case one of the promises fails so I went with Promise.all: 025e4c1
| } | ||
|
|
||
| /** | ||
| * Looks for a particular string pattern (`sdbid-[debug ID]`) in the bundle |
There was a problem hiding this comment.
Just wanted to use something short. It can be changed at any time since nothing depends on this. Do you think another pattern fits better?
There was a problem hiding this comment.
I would rather it be sentry, but it's no big deal.
There was a problem hiding this comment.
I would rather it be sentry, but it's no big deal.
No! good point! :) befc007
AbhiPrasad
left a comment
AbhiPrasad
left a comment
There was a problem hiding this comment.
let's ship and try testing it out!
|
|
||
| describe("stringToUUID", () => { | ||
| test("should return a deterministic UUID", () => { | ||
| expect(stringToUUID("Nothing personnel kid")).toBe("52c7a762-5ddf-49a7-6f16-6874a8cb2512"); |
|
|
||
| // For now we write both fields until we know what will become the standard - if ever. | ||
| map["debug_id"] = debugId; | ||
| map["debugId"] = debugId; |
2 participants
Ref #191
This PR adds an experimental option to the bundler plugins that will upload source files with debug IDs to Sentry.
In short, the following steps are executed:
//# debugId=...comment at the bottom of the chunks and write the chunk to the temp folder.debugIdanddebug_idfield to the corresponding source maps if they are found and write them to the temp folder.