fix(bundler-plugins): do not warn on known sourcemap-less chunks#22267
fix(bundler-plugins): do not warn on known sourcemap-less chunks#22267isaacs wants to merge 2 commits into
Conversation
|
This warning highlighted to users if they don't have sourcemaps enabled. After these changes is this still some kind of warning in this case? |
Ah, yes, that would be bad, whoops. Good catch. I think we can still resolve this, though, by moving the warning up a level, to the manager instead of on the per-file level. Fixed on bfc2b32 |
|
The test failures suggest that this change results in the sourcemap cli command no longer running when there are no sourcemaps. I guess this is preferable but I don't fully understand the consequences! |
| if (files.length === 0) { | ||
| logger.warn( | ||
| `No source maps found for any of the ${debugIdChunkFilePaths.length} matched build ` + | ||
| 'artifacts, so no source maps were uploaded to Sentry. This usually means source map ' + | ||
| 'generation is not enabled in your bundler. Enable it so Sentry can un-minify your stack traces.', | ||
| ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Bug: The warning for no source maps being found is misleading. It assumes source map generation is disabled, but it could also mean the asset glob only matched chunks without source maps.
Severity: LOW
Suggested Fix
The warning message should be more nuanced. Instead of definitively stating that source map generation is disabled, it should suggest that either source map generation is disabled OR that the sourcemaps.assets glob pattern might not be matching any files that have source maps. This would guide the user to check both their bundler configuration and their plugin configuration.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/bundler-plugins/src/core/build-plugin-manager.ts#L710-L717
Potential issue: The warning logged when no source maps are found for upload is
misleading. The code checks if the temporary upload folder is empty (`files.length ===
0`) and, if so, logs that "source map generation is not enabled". However, this folder
can also be empty when the `sourcemaps.assets` glob pattern correctly matches files that
legitimately do not have source maps, such as framework stubs. This can misdirect users
into debugging their bundler configuration when the actual issue is a misconfigured glob
pattern that only includes sourcemap-less chunks.
Did we get this right? 👍 / 👎 to inform future reviews.
Prevent potentially large number of noisy warnings that by design never emit sourcemaps. For example: ``` ~/b0f41f6b-e36f-409b-817f-921f7d295a7b-155.js (no sourcemap found, debug id b0f41f6b-...) - warning: could not determine a source map reference (Could not auto-detect referenced sourcemap for ~/b0f41f6b-...-155.js) ``` This cannot be safely avoided using the file path glob approach, as seen in #18938, because the file paths in question here (uuids and alphanumeric hashes) can't be guaranteed to _never_ hold user code as seen in the cases covered by #18938. Instead, when no symbolication can be performed because the chunk is a stub without any sourcemap, downgrade the warning to a debug log. fix: #22199 fix: JS-3006
bfc2b32 to
9ed5e0b
Compare
Prevent potentially large number of noisy warnings that by design never emit sourcemaps.
For example:
This cannot be safely avoided using the file path glob approach, as seen in #18938, because the file paths in question here (uuids and alphanumeric hashes) can't be guaranteed to never hold user code as seen in the cases covered by #18938.
Instead, when no symbolication can be performed because the chunk is a stub without any sourcemap, downgrade the warning to a debug log.
fix: #22199
fix: JS-3006