-
-
Notifications
You must be signed in to change notification settings - Fork 20
feat: add source map support for production stack trace rewriting #284
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?
Changes from all commits
71e5d9f
6a944a6
8bacc49
bbb4b62
275a497
45b9569
0de6b4a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import { existsSync, readFileSync } from "node:fs"; | ||
| import { createRequire, register } from "node:module"; | ||
| import { join, relative } from "node:path"; | ||
| import { pathToFileURL } from "node:url"; | ||
| import { fileURLToPath, pathToFileURL } from "node:url"; | ||
|
|
||
| import { init$ as cache_init$, useCache } from "../../cache/index.mjs"; | ||
| import { context$, ContextStorage, getContext } from "../../server/context.mjs"; | ||
|
|
@@ -83,12 +84,16 @@ export default async function ssrHandler(root, options = {}) { | |
| } | ||
| const [ | ||
| { render }, | ||
| { default: buildMetadata }, | ||
| { default: Component, init$: root_init$ }, | ||
| { default: GlobalErrorComponent }, | ||
| { default: ErrorBoundary }, | ||
| rscSerializer, | ||
| ] = await Promise.all([ | ||
| import(pathToFileURL(entryModule)), | ||
| import(pathToFileURL(join(cwd, options.outDir, "server/build-meta.json")), { | ||
| with: { type: "json" }, | ||
| }), | ||
| import(pathToFileURL(rootModule)), | ||
| import(pathToFileURL(globalErrorModule)), | ||
| errorBoundary | ||
|
|
@@ -109,6 +114,34 @@ export default async function ssrHandler(root, options = {}) { | |
| const moduleCacheStorage = new ContextManager(); | ||
| await module_loader_init$(moduleLoader, moduleCacheStorage); | ||
|
|
||
| if (buildMetadata.sourcemap && buildMetadata.sourcemap !== false) { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As this is now related to manifests, I would move this into https://github.com/lazarv/react-server/blob/main/packages/react-server/lib/start/manifest.mjs#L52-L56 and also store it in the context. |
||
| const { default: sourceMapSupport } = await import("source-map-support"); | ||
| sourceMapSupport.install({ | ||
| environment: "node", | ||
| hookRequire: buildMetadata.sourcemap === "inline", | ||
| handleUncaughtExceptions: false, | ||
| retrieveSourceMap: | ||
| buildMetadata.sourcemap === "hidden" | ||
| ? (source) => { | ||
| logger.info(`Retrieving source map for ${source}`); | ||
| if (source.startsWith("file:")) { | ||
| const mapFilePath = fileURLToPath(source) + ".map"; | ||
| if (existsSync(mapFilePath)) { | ||
| return { | ||
| url: source, | ||
| map: readFileSync(mapFilePath, "utf8"), | ||
| }; | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
| : undefined, | ||
| }); | ||
| logger.info( | ||
| `Source map (${buildMetadata.sourcemap === true ? "file" : buildMetadata.sourcemap}) stack trace mapping support enabled` | ||
| ); | ||
| } | ||
|
|
||
| const importMap = | ||
| configRoot.importMap || configRoot.resolve?.shared | ||
| ? { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,6 +52,34 @@ import { | |
| import { ServerFunctionNotFoundError } from "./action-state.mjs"; | ||
| import { cwd } from "../lib/sys.mjs"; | ||
|
|
||
| /** | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This wrapper could be moved to https://github.com/lazarv/react-server/blob/main/packages/react-server/lib/start/create-logger.mjs, so that the logger would use this everywhere by default. Also, the wrapper should only be enabled when using source maps. |
||
| * Helper to log errors with source-mapped stack traces. | ||
| * Forces Error.prepareStackTrace to run if installed by source-map-support. | ||
| */ | ||
| function logErrorWithStackTrace(logger, error) { | ||
| if (!error) return; | ||
| if ( | ||
| error.stack.startsWith( | ||
| "Error: An error occurred in the Server Components render." | ||
| ) | ||
| ) | ||
| return; | ||
|
|
||
| // Access .stack to trigger Error.prepareStackTrace if it's been set | ||
| // source-map-support hooks into this to provide mapped stack traces | ||
| const stack = error.stack; | ||
|
|
||
| // Log the error with its prepared stack trace | ||
| if (logger?.error) { | ||
| logger.error(error); | ||
| } else { | ||
| console.error(error); | ||
| } | ||
|
|
||
| // Return the stack for potential further processing | ||
| return stack; | ||
| } | ||
|
|
||
| const serverReferenceMap = new Proxy( | ||
| {}, | ||
| { | ||
|
|
@@ -522,6 +550,10 @@ export async function render(Component, props = {}, options = {}) { | |
| temporaryReferences, | ||
| onError(e) { | ||
| hasError = true; | ||
| // Log error with source-mapped stack trace | ||
| if (import.meta.env.PROD) { | ||
| logErrorWithStackTrace(logger, e); | ||
| } | ||
| const redirect = getContext(REDIRECT_CONTEXT); | ||
| if (redirect?.response) { | ||
| return `Location=${redirect.response.headers.get("location")}`; | ||
|
|
@@ -660,6 +692,10 @@ export async function render(Component, props = {}, options = {}) { | |
| temporaryReferences, | ||
| onError(e) { | ||
| hasError = true; | ||
| // Log error with source-mapped stack trace | ||
| if (import.meta.env.PROD) { | ||
| logErrorWithStackTrace(logger, e); | ||
| } | ||
| const redirect = getContext(REDIRECT_CONTEXT); | ||
| if (redirect?.response) { | ||
| return resolve(redirect.response); | ||
|
|
@@ -763,7 +799,8 @@ export async function render(Component, props = {}, options = {}) { | |
| if (!e.digest && digest) { | ||
| e.digest = digest; | ||
| } | ||
| (logger ?? console).error(e); | ||
| // Log error with source-mapped stack trace | ||
| logErrorWithStackTrace(logger, e); | ||
| hasError = true; | ||
| if (!isStarted) { | ||
| ContextStorage.run(contextStore, async () => { | ||
|
|
@@ -822,13 +859,15 @@ export async function render(Component, props = {}, options = {}) { | |
| ); | ||
| } | ||
| } catch (e) { | ||
| logger.error(e); | ||
| // Log error with source-mapped stack trace | ||
| logErrorWithStackTrace(logger, e); | ||
| getContext(ERROR_CONTEXT)?.(e)?.then(resolve, reject); | ||
| } | ||
| }); | ||
| return streaming; | ||
| } catch (e) { | ||
| logger.error(e); | ||
| // Log error with source-mapped stack trace | ||
| logErrorWithStackTrace(logger, e); | ||
| return new Response(null, { | ||
| status: 500, | ||
| statusText: "Internal Server Error", | ||
|
|
||
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.
As all other manifests have the
server/<type>-manifest.jsonname, I would suggest to have this also namedserver/build-manifest.json.