Skip to content

Commit

Permalink
Log uncaught source-mapped errors to the console (#446)
Browse files Browse the repository at this point in the history
Previously, errors _thrown_ using the `MF-Experimental-Error-Stack`
header were only shown in a pretty-error page. This PR logs those
errors with source-maps applied to the console too, using the
`source-map-support` package. This simplifies our code, and also
means we don't need to touch internal Youch methods, as the errors
we pass to Youch are already source-mapped.
  • Loading branch information
mrbbot committed Oct 31, 2023
1 parent 555187a commit 592e278
Show file tree
Hide file tree
Showing 6 changed files with 265 additions and 124 deletions.
3 changes: 2 additions & 1 deletion packages/miniflare/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"glob-to-regexp": "^0.4.1",
"http-cache-semantics": "^4.1.0",
"kleur": "^4.1.5",
"source-map": "^0.7.4",
"source-map-support": "0.5.21",
"stoppable": "^1.1.0",
"undici": "^5.12.0",
"workerd": "^1.20221111.5",
Expand All @@ -49,6 +49,7 @@
"@types/estree": "^1.0.0",
"@types/glob-to-regexp": "^0.4.1",
"@types/http-cache-semantics": "^4.0.1",
"@types/source-map-support": "^0.5.6",
"@types/stoppable": "^1.1.1",
"@types/ws": "^8.5.3"
},
Expand Down
6 changes: 5 additions & 1 deletion packages/miniflare/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,11 @@ export class Miniflare {
const workerSrcOpts = this.#workerOpts.map<SourceOptions>(
({ core }) => core
);
response = await handlePrettyErrorRequest(workerSrcOpts, request);
response = await handlePrettyErrorRequest(
this.#log,
workerSrcOpts,
request
);
} else {
// TODO: check for proxying/outbound fetch header first (with plans for fetch mocking)
response = await this.#handleLoopbackPlugins(request, url);
Expand Down
157 changes: 157 additions & 0 deletions packages/miniflare/src/plugins/core/errors/callsite.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
// Lifted from `node-stack-trace`:
// https://github.com/felixge/node-stack-trace/blob/4c41a4526e74470179b3b6dd5d75191ca8c56c17/index.js
// Ideally, we'd just use this package as-is, but it has a strict
// `engines.node == 16` constraint in its `package.json`. There's a PR open to
// fix this (https://github.com/felixge/node-stack-trace/pull/39), but it's been
// open for a while. As soon as it's merged, we should just depend on it.

/*!
* Copyright (c) 2011 Felix Geisendörfer ([email protected])
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

export function parseStack(stack: string): CallSite[] {
return stack
.split("\n")
.slice(1)
.map(parseCallSite)
.filter((site): site is CallSite => site !== undefined);
}

function parseCallSite(line: string): CallSite | undefined {
const lineMatch = line.match(
/at (?:(.+?)\s+\()?(?:(.+?):(\d+)(?::(\d+))?|([^)]+))\)?/
);
if (!lineMatch) {
return;
}

let object = null;
let method = null;
let functionName = null;
let typeName = null;
let methodName = null;
const isNative = lineMatch[5] === "native";

if (lineMatch[1]) {
functionName = lineMatch[1];
let methodStart = functionName.lastIndexOf(".");
if (functionName[methodStart - 1] == ".") methodStart--;
if (methodStart > 0) {
object = functionName.substring(0, methodStart);
method = functionName.substring(methodStart + 1);
const objectEnd = object.indexOf(".Module");
if (objectEnd > 0) {
functionName = functionName.substring(objectEnd + 1);
object = object.substring(0, objectEnd);
}
}
}

if (method) {
typeName = object;
methodName = method;
}

if (method === "<anonymous>") {
methodName = null;
functionName = null;
}

return new CallSite({
typeName,
functionName,
methodName,
fileName: lineMatch[2] || null,
lineNumber: parseInt(lineMatch[3]) || null,
columnNumber: parseInt(lineMatch[4]) || null,
native: isNative,
});
}

export interface CallSiteOptions {
typeName: string | null;
functionName: string | null;
methodName: string | null;
fileName: string | null;
lineNumber: number | null;
columnNumber: number | null;
native: boolean;
}

// https://v8.dev/docs/stack-trace-api#customizing-stack-traces
// This class supports the subset of options implemented by `node-stack-trace`:
// https://github.com/felixge/node-stack-trace/blob/4c41a4526e74470179b3b6dd5d75191ca8c56c17/index.js
export class CallSite implements NodeJS.CallSite {
constructor(private readonly opts: CallSiteOptions) {}

getThis(): unknown {
return null;
}
getTypeName(): string | null {
return this.opts.typeName;
}
// eslint-disable-next-line @typescript-eslint/ban-types
getFunction(): Function | undefined {
return undefined;
}
getFunctionName(): string | null {
return this.opts.functionName;
}
getMethodName(): string | null {
return this.opts.methodName;
}
getFileName(): string | null {
return this.opts.fileName;
}
getLineNumber(): number | null {
return this.opts.lineNumber;
}
getColumnNumber(): number | null {
return this.opts.columnNumber;
}
getEvalOrigin(): string | undefined {
return undefined;
}
isToplevel(): boolean {
return false;
}
isEval(): boolean {
return false;
}
isNative(): boolean {
return this.opts.native;
}
isConstructor(): boolean {
return false;
}
isAsync(): boolean {
return false;
}
isPromiseAll(): boolean {
return false;
}
isPromiseAny(): boolean {
return false;
}
getPromiseIndex(): number | null {
return null;
}
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import fs from "fs";
import path from "path";
import { fileURLToPath } from "url";
import { SourceMapConsumer } from "source-map";
import type { Entry } from "stacktracey";
import type { UrlAndMap } from "source-map-support";
import { Request, Response } from "undici";
import { z } from "zod";
import { Log } from "../../../shared";
import {
ModuleDefinition,
contentsToString,
maybeGetStringScriptPathIndex,
} from "./modules";
} from "../modules";
import { getSourceMapper } from "./sourcemap";

// Subset of core worker options that define Worker source code.
// These are the possible cases, and corresponding reported source files in
Expand Down Expand Up @@ -163,85 +163,30 @@ function maybeGetFile(
return maybeGetDiskFile(filePath);
}

// Try to find a source map for `sourceFile`, and update `sourceFile` and
// `frame` with source mapped locations
type SourceMapCache = Map<string, Promise<SourceMapConsumer>>;
async function applySourceMaps(
/* mut */ sourceMapCache: SourceMapCache,
/* mut */ sourceFile: SourceFile,
/* mut */ frame: Entry
) {
// If we don't have a file path, or full location, we can't do any source
// mapping, so return straight away.
// TODO: support data URIs for maps, in `sourceFile`s without path
if (
sourceFile.path === undefined ||
frame.line === undefined ||
frame.column === undefined
) {
return;
}

// Find the last source mapping URL if any
const sourceMapRegexp = /# sourceMappingURL=(.+)/g;
let sourceMapMatch: RegExpMatchArray | null = null;
while (true) {
const match = sourceMapRegexp.exec(sourceFile.contents);
if (match !== null) sourceMapMatch = match;
else break;
}
// If we couldn't find a source mapping URL, there's nothing we can do
if (sourceMapMatch === null) return;

// Get the source map
const root = path.dirname(sourceFile.path);
const sourceMapPath = path.resolve(root, sourceMapMatch[1]);
let consumerPromise = sourceMapCache.get(sourceMapPath);
if (consumerPromise === undefined) {
// If we couldn't find the source map in cache, load it
function getSourceMappedStack(workerSrcOpts: SourceOptions[], error: Error) {
// This function needs to match the signature of the `retrieveSourceMap`
// option from the "source-map-support" package.
function retrieveSourceMap(file: string): UrlAndMap | null {
const sourceFile = maybeGetFile(workerSrcOpts, file);
if (sourceFile?.path === undefined) return null;

// Find the last source mapping URL if any
const sourceMapRegexp = /# sourceMappingURL=(.+)/g;
const matches = [...sourceFile.contents.matchAll(sourceMapRegexp)];
// If we couldn't find a source mapping URL, there's nothing we can do
if (matches.length === 0) return null;
const sourceMapMatch = matches[matches.length - 1];

// Get the source map
const root = path.dirname(sourceFile.path);
const sourceMapPath = path.resolve(root, sourceMapMatch[1]);
const sourceMapFile = maybeGetDiskFile(sourceMapPath);
if (sourceMapFile === undefined) return;
const rawSourceMap = JSON.parse(sourceMapFile.contents);
consumerPromise = new SourceMapConsumer(rawSourceMap);
sourceMapCache.set(sourceMapPath, consumerPromise);
}
const consumer = await consumerPromise;
if (sourceMapFile === undefined) return null;

// Get original position from source map
const original = consumer.originalPositionFor({
line: frame.line,
column: frame.column,
});
// If source mapping failed, don't make changes
if (
original.source === null ||
original.line === null ||
original.column === null
) {
return;
return { map: sourceMapFile.contents };
}

// Update source file and frame with source mapped locations
const newSourceFile = maybeGetDiskFile(original.source);
if (newSourceFile === undefined) return;
sourceFile.path = original.source;
sourceFile.contents = newSourceFile.contents;
frame.file = original.source;
frame.fileRelative = path.relative("", sourceFile.path);
frame.fileShort = frame.fileRelative;
frame.fileName = path.basename(sourceFile.path);
frame.line = original.line;
frame.column = original.column;
}

interface YouchInternalFrameSource {
pre: string[];
line: string;
post: string[];
}
interface YouchInternals {
options: { preLines: number; postLines: number };
_getFrameSource(frame: Entry): Promise<YouchInternalFrameSource | null>;
return getSourceMapper()(retrieveSourceMap, error);
}

// Due to a bug in `workerd`, if `Promise`s returned from native C++ APIs are
Expand All @@ -257,6 +202,7 @@ const ErrorSchema = z.object({
stack: z.ostring(),
});
export async function handlePrettyErrorRequest(
log: Log,
workerSrcOpts: SourceOptions[],
request: Request
): Promise<Response> {
Expand All @@ -271,11 +217,11 @@ export async function handlePrettyErrorRequest(
error.message = caught.message as string;
error.stack = caught.stack;

// Create a source-map cache for this pretty-error request. We only cache per
// request, as it's likely the user will update their code and restart/call
// `setOptions` again on seeing this page. This would invalidate existing
// source maps. Keeping the cache per request simplifies things too.
const sourceMapCache: SourceMapCache = new Map();
// Try to apply source-mapping
error.stack = getSourceMappedStack(workerSrcOpts, error);

// Log source-mapped error to console if logging enabled
log.error(error);

// Lazily import `youch` when required
const Youch: typeof import("youch").default = require("youch");
Expand All @@ -290,40 +236,7 @@ export async function handlePrettyErrorRequest(
'<a href="https://discord.gg/cloudflaredev" target="_blank" style="text-decoration:none">💬 Workers Discord</a>',
].join("");
});
const youchInternals = youch as unknown as YouchInternals;
youchInternals._getFrameSource = async (frame) => {
// Adapted from Youch's own implementation
let file = frame.file
.replace(/dist\/webpack:\//g, "") // Unix
.replace(/dist\\webpack:\\/g, ""); // Windows
// Ignore error as frame source is optional anyway
try {
file = file.startsWith("file:") ? fileURLToPath(file) : file;
} catch {}

// Try get source-mapped file contents
const sourceFile = await maybeGetFile(workerSrcOpts, file);
if (sourceFile === undefined || frame.line === undefined) return null;
// If source-mapping fails, this function won't do anything
await applySourceMaps(sourceMapCache, sourceFile, frame);

// Return lines around frame as required by Youch
const lines = sourceFile.contents.split(/\r?\n/);
const line = frame.line;
const pre = lines.slice(
Math.max(0, line - (youchInternals.options.preLines + 1)),
line - 1
);
const post = lines.slice(line, line + youchInternals.options.postLines);
return { pre, line: lines[line - 1], post };
};

try {
return new Response(await youch.toHTML(), {
headers: { "Content-Type": "text/html;charset=utf-8" },
});
} finally {
// Clean-up source-map cache
for (const consumer of sourceMapCache.values()) (await consumer).destroy();
}
return new Response(await youch.toHTML(), {
headers: { "Content-Type": "text/html;charset=utf-8" },
});
}
Loading

0 comments on commit 592e278

Please sign in to comment.