-
-
Notifications
You must be signed in to change notification settings - Fork 7k
feat: collect errors from the browser #20487
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
base: main
Are you sure you want to change the base?
Conversation
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.
Thanks.
- communication
- I think we can communicate via the HotChannel (e.g.
transport.send({ type: 'clientError' })
) instead of a fetch request. This way we can re-use the same code when we send error information from other environments.
- I think we can communicate via the HotChannel (e.g.
- error collection
- I think we should listen the
unhandledrejection
event as well to catch the unhandled promise rejections.
- I think we should listen the
- codeframe generation
generateCodeFrame
function can be used for the code frame generation.fetch(info.filename)
can be probably replaced withmoduleGraph.getModuleByUrl(id).transformResult
- [optional] It seems
colno
andlineno
contains the position for the transformed script. I think we need to map that back to the original code. That should be possible by a code similar tovite/packages/vite/src/node/ssr/ssrStacktrace.ts
Lines 36 to 49 in f20addc
const mod = moduleGraph.getModuleById(id) const rawSourceMap = mod?.transformResult?.map if (!rawSourceMap) { return input } const traced = new TraceMap(rawSourceMap as any) const pos = originalPositionFor(traced, { line: Number(line) - offset, // stacktrace's column is 1-indexed, but sourcemap's one is 0-indexed column: Number(column) - 1, }) - I think we can remove the code highlight feature for the following reasons:
- Since this feature is mainly for LLMs, it doesn't help much.
- We don't have that in other places.
@shijikijs/cli
is huge and will made Vite's install size to be 1.5x.
We also need a new option |
@@ -954,7 +958,7 @@ export async function _createServer( | |||
} | |||
|
|||
// error handler | |||
middlewares.use(errorMiddleware(server, !!middlewareMode)) |
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.
This removal was accidental, will fix it!
ideally the location logged on the terminal console uses |
+1 to using the websocket, that would also get rid of the body-parser dependency. |
try { | ||
new Function('throw new Error(1)')() | ||
} catch (e) { | ||
// in Node 12, stack traces account for the function wrapper. |
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 don't think we have to support node12 here, our engine field asks for 20.19+
This adds a browser error handler and associated error formatting.
There's on unsolved issue though: when accessing
transformResult
, it seemsmap
is alwaysnull
.