-
Notifications
You must be signed in to change notification settings - Fork 158
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: support overlay display unhandled runtime errors #2310
Open
nanianlisao
wants to merge
4
commits into
web-infra-dev:main
Choose a base branch
from
nanianlisao:feat_runtime_error_overlay
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+359
−41
Open
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,7 +58,8 @@ | |
"@swc/helpers": "0.5.3", | ||
"core-js": "~3.36.0", | ||
"html-webpack-plugin": "npm:[email protected]", | ||
"postcss": "^8.4.38" | ||
"postcss": "^8.4.38", | ||
"source-map": "0.5.7" | ||
}, | ||
"devDependencies": { | ||
"@types/node": "18.x", | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
// @ts-expect-error | ||
import { SourceMapConsumer } from 'source-map'; | ||
|
||
const fetchContent = (url: string) => fetch(url).then((r) => r.text()); | ||
|
||
const findSourceMap = async (fileSource: string, filename: string) => { | ||
try { | ||
// Prefer to get it via filename + '.map'. | ||
const mapUrl = `${filename}.map`; | ||
return await fetchContent(mapUrl); | ||
} catch (e) { | ||
const mapUrl = fileSource.match(/\/\/# sourceMappingURL=(.*)$/)?.[1]; | ||
if (mapUrl) return await fetchContent(mapUrl); | ||
} | ||
}; | ||
|
||
// Format line numbers to ensure alignment | ||
const parseLineNumber = (start: number, end: number) => { | ||
const digit = Math.max(start.toString().length, end.toString().length); | ||
return (line: number) => line.toString().padStart(digit); | ||
}; | ||
|
||
// Escapes html tags to prevent them from being parsed in pre tags | ||
const escapeHTML = (str: string) => | ||
str | ||
.replace(/&/g, '&') | ||
.replace(/</g, '<') | ||
.replace(/>/g, '>') | ||
.replace(/"/g, '"') | ||
.replace(/'/g, '''); | ||
|
||
// Based on the sourceMap information, beautify the source code and mark the error lines | ||
const formatSourceCode = (sourceCode: string, pos: any) => { | ||
// Note that the line starts at 1, not 0. | ||
const { line: crtLine, column, name } = pos; | ||
const lines = sourceCode.split('\n'); | ||
|
||
// Display up to 6 lines of source code | ||
const lineCount = Math.min(lines.length, 6); | ||
const result = []; | ||
|
||
const startLine = Math.max(1, crtLine - 2); | ||
const endLine = Math.min(startLine + lineCount - 1, lines.length); | ||
|
||
const parse = parseLineNumber(startLine, endLine); | ||
|
||
for (let line = startLine; line <= endLine; line++) { | ||
const prefix = `${line === crtLine ? '->' : ' '} ${parse(line)} | `; | ||
const lineCode = escapeHTML(lines[line - 1] ?? ''); | ||
result.push(prefix + lineCode); | ||
|
||
// When the sourcemap information includes specific column details, add an error hint below the error line. | ||
if (line === crtLine && column > 0) { | ||
const errorLine = `${' '.repeat(prefix.length + column)}<span style="color: #fc5e5e;">${'^'.repeat(name?.length || 1)}</span>`; | ||
result.push(errorLine); | ||
} | ||
} | ||
|
||
return result.filter(Boolean).join('\n'); | ||
}; | ||
|
||
// Try to find the source based on the sourceMap information. | ||
export const findSourceCode = async (sourceInfo: any) => { | ||
const { filename, line, column } = sourceInfo; | ||
const fileSource = await fetch(filename).then((r) => r.text()); | ||
|
||
const smContent = await findSourceMap(fileSource, filename); | ||
|
||
if (!smContent) return; | ||
const rawSourceMap = JSON.parse(smContent); | ||
|
||
const consumer = await new SourceMapConsumer(rawSourceMap); | ||
|
||
// Use sourcemap to find the source code location | ||
const pos = consumer.originalPositionFor({ | ||
line: Number.parseInt(line, 10), | ||
column: Number.parseInt(column, 10), | ||
}); | ||
|
||
const url = `${pos.source}:${pos.line}:${pos.column}`; | ||
const sourceCode = consumer.sourceContentFor(pos.source); | ||
return { | ||
sourceCode: formatSourceCode(sourceCode, pos), | ||
sourceFile: url, | ||
}; | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I think import
source-map
will resolve to the Node.js version and get an error like this: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.
And we do not want to add third-party dependencies to
@rsbuild/core
, unless it can be prebundled to thecompiled
folder.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.
Yes, I tried to upgrade
source-map
to 0.7.4 and ran into some problems. It seems to be a problem with thesource-map
package itself.Do you think it would work if I prebundled
[email protected]
into the compiled directory? If yes, I'll try to do it this wayThere 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.
No,
[email protected]
was released 7 years ago and we won't use a legacy version..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.
However, version
0.7.4
has a major problem that makes it unusable on the browser side. I don't think0.5.7
should be rejected just because it's outdated, but whether it's more important to consider its usability.I have found no good solution to this problem without changing the version. mozilla/source-map#459
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.
Maybe we can use source-map-js.
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.
@9aoy Thanks for sharing.
I have a new problem. When I prebundler, I get an error:
__filename is not defined
.I didn't find a good solution and had to improvise a less elegant solution.
The esm polyfill for
__dirname
is mostly Node-based. Sincerely hope to find a better solution.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.
The prebundle is used to bundle Node.js packages, and
source-map-js
in imported in the client code, so it can not be prebundled.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.
For the client code, I think the new code added in the PR should be imported on demand.
In other words, when the user does not enable the runtime error overlay, these code and
source-map-js
should not be bundled into the client code, otherwise it will introduce unused code and slow down the build.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.
Okay, let's take a look