clarifications

Author: Swatinem

text/0081-sourcemap-debugid.md

@@ -93,11 +93,17 @@ This creates a new `DebugId` by hashing the contents of the JavaScript file.
 
 ### Based on SourceMap Content-hash
 
-This creates a new `DebugId` by hashing the contents of the SourceMap file.
+This creates a new `DebugId` by hashing the contents of the SourceMap file. The reasoning for this is primarily
+motivated by a drawback of the previous option.
+It is possible that non-essential changes to the source file, such as whitespace or comments, will be completely
+removed by a minifier, and will result in an identical minified output file.
+These changes would however create different mappings in the SourceMap file, making it possible to resolve to the
+correct original source.
 
 **pros**
 
-- Generates a new `DebugId` for changes to source files that would otherwise not lead to changes in the JavaScript file.
+- Generates a new `DebugId` for changes to source files that would otherwise not lead to changes in the (minified) JavaScript file.
+- SourceMap processing will always resolve back to the original source code.
 
 **cons**
 
@@ -147,13 +153,31 @@ It is therefore necessary to extract this `DebugId` through other means.
 Current JavaScript stack traces include the absolute path (called `abs_path`) of each stack frame. It should be possible
 to load and inspect that file at runtime whenever an error happens.
 
+An example of this might look like this:
+
+```js
+// cached in the SDK:
+const RESOLVED_FRAMES = new Map();
+async function attachDebugMeta(event) {
+  for (const { abs_path } of allStackFrames(event)) {
+    if (!RESOLVED_FRAMES.has(abs_path)) {
+      const rawSource = await fetch(abs_path).then((res) => res.text());
+      RESOLVED_FRAMES.set(abs_path, extractDebugIdFromSource(rawSource));
+    }
+  }
+
+  event.debug_meta = resolvedFramesToImages(RESOLVED_FRAMES);
+}
+```
+
 **pros**
 
 - Does not require injecting any _code_ into the JavaScript files.
 
 **cons**
 
-- Might incur some async fetching / IO when capturing an Error. Though any `abs_path` in the stack trace should be cached already.
+- Needs `async` code to resolve `DebugId`s, and might incur some async fetching / IO when capturing an Error.
+- Though any source referenced from the stack trace via `abs_path` is likely in the browser cache already.
 
 #### Add the `DebugId` to a global at load time
 
@@ -162,7 +186,8 @@ the `DebugId` to a global map.
 
 An example snippet is here:
 
-```
+<!-- prettier-ignore -->
+```js
 !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]="XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX")}catch(e){}}()
 ```
 
@@ -175,12 +200,14 @@ Further post-processing at time of capturing an `Error` is required to extract t
 
 **cons**
 
-- It does however require parsing of the `Error.stack` at time of capturing the `Error`.
+- It does however require parsing of the `Error.stack`s in `_sentryDebugIds` at time of capturing the `Error`.
+- However this should be cached and only happen once.
 
 An alternative implementation might use the [`import.meta.url`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import.meta)
 property. This would avoid capturing and post-processing an `Error.stack`, but does require usage of ECMAScript Modules.
 
-```
+<!-- prettier-ignore -->
+```js
 ((globalThis._sentryDebugIds=globalThis._sentryDebugIds||{})[import.meta.url]="XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX");
 ```
 
@@ -214,8 +241,11 @@ In this example, assets are _fingerprinted_, and after being fully propagated
 through a global CDN, they are starting to be referenced from the backend
 service via HTML.
 
-This may work with unique content-hash based filenames, and even use _fingerprinting_ and
-[Subresource Integrity (SRI)](https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity).
+_Fingerprinting_ in this case means creating a unique content-hash which is then
+used in various of ways:
+
+- As part of the filename, to give each file a unique and stable filename.
+- Use the derived hash for [Subresource Integrity (SRI)](https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity).
 
 An example may look like this, for a CDN-deployed and fingerprinted reference
 to [katex](https://katex.org/docs/browser.html#starter-template):
@@ -232,7 +262,18 @@ to [katex](https://katex.org/docs/browser.html#starter-template):
 Not only is the deployment pipeline very complex, it can also involve a variety of tools with varying degree of
 integration between them.
 The example `<script>` tag shown above might be generated as part of one integrated JS bundler tool, or it might be
-generated by a Rust or python backend, based on supplied JSON file.
+generated by a Rust or python backend, based on a supplied JSON file, which might look like this:
+
+```json
+{
+  "assets": {
+    "assert_id": {
+      "filename": "my_asset.abcdefg.min.js",
+      "integrity": "sha384-VWXYZ"
+    }
+  }
+}
+```
 
 The checksums themselves might be directly output by a JS bundler tool, or they might be generated by a completely
 different tool at another stage of the build pipeline.
@@ -272,13 +313,15 @@ In this scenario, injection happens at the time of `sentry-cli upload`, and will
 
 **cons**
 
+- Violates expectations of file immutability and integrity.
 - Does not work with bundlers that integrate fingerprinting.
 - Does not work in build pipelines where `sentry-cli upload` is not in the main deployment path.
 
 ### Injection via bundler plugins
 
-Here, we would build `DebugId` injection right into the various JavaScript bundlers. This can happen with a third-party
-plugin at first, and might move into the core bundler packages once there is enough community buy-in for `DebugId`s.
+Here, we would build `DebugId` injection right into the various JavaScript bundlers.
+We should validate the core ideas using third-party plugins at first, and then strive for inclusion in the
+upstream bundlers directly.
 
 Each bundler is unique though, and has different hooks at different stages of its internal pipeline. Some bundlers
 might not have the necessary hooks at the necessary stage at all.
@@ -358,5 +401,6 @@ It also has a `manifest.json`, which has more metadata per file, like the type o
 
 **cons**
 
-- Does not work well content-hash based `DebugId`s, as one `DebugId` can appear in a multitude of archives.
+- Does not work well with content-hash based `DebugId`s, as one `DebugId` can appear in a multitude of archives.
+  Picking the one archive that covers all needed files is non-trivial.
 - Feels like a workaround for inefficiencies in other parts of the processing pipeline when dealing with more smaller files.