diff --git a/src/jsc/bindings/CallSitePrototype.cpp b/src/jsc/bindings/CallSitePrototype.cpp index 5a537ecccb62..489626334430 100644 --- a/src/jsc/bindings/CallSitePrototype.cpp +++ b/src/jsc/bindings/CallSitePrototype.cpp @@ -134,15 +134,22 @@ JSC_DEFINE_HOST_FUNCTION(callSiteProtoFuncGetFileName, (JSGlobalObject * globalO JSC_DEFINE_HOST_FUNCTION(callSiteProtoFuncGetLineNumber, (JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) { ENTER_PROTO_FUNC(); - // https://github.com/mozilla/source-map/blob/60adcb064bf033702d954d6d3f9bc3635dcb744b/lib/source-map-consumer.js#L484-L486 - return JSC::JSValue::encode(jsNumber(std::max(callSite->lineNumber().oneBasedInt(), 1))); + auto line = callSite->lineNumber(); + if (line == OrdinalNumber::beforeFirst()) + return JSC::JSValue::encode(JSC::jsNull()); + return JSC::JSValue::encode(jsNumber(line.oneBasedInt())); } +// V8's CallSite#getColumnNumber() is 1-based (https://v8.dev/docs/stack-trace-api). It returns null +// when no position is available. source-map-support relies on the 1-based value to compute a +// 0-based offset for originalPositionFor(). JSC_DEFINE_HOST_FUNCTION(callSiteProtoFuncGetColumnNumber, (JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) { ENTER_PROTO_FUNC(); - // https://github.com/mozilla/source-map/blob/60adcb064bf033702d954d6d3f9bc3635dcb744b/lib/source-map-consumer.js#L488-L489 - return JSC::JSValue::encode(jsNumber(std::max(callSite->columnNumber().zeroBasedInt(), 0))); + auto column = callSite->columnNumber(); + if (column == OrdinalNumber::beforeFirst()) + return JSC::JSValue::encode(JSC::jsNull()); + return JSC::JSValue::encode(jsNumber(column.oneBasedInt())); } // TODO: @@ -256,9 +263,11 @@ JSC_DEFINE_HOST_FUNCTION(callSiteProtoFuncToJSON, (JSGlobalObject * globalObject { ENTER_PROTO_FUNC(); JSObject* obj = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 4); + auto line = callSite->lineNumber(); + auto column = callSite->columnNumber(); obj->putDirect(vm, JSC::Identifier::fromString(vm, "sourceURL"_s), callSite->sourceURL()); - obj->putDirect(vm, JSC::Identifier::fromString(vm, "lineNumber"_s), jsNumber(callSite->lineNumber().oneBasedInt())); - obj->putDirect(vm, JSC::Identifier::fromString(vm, "columnNumber"_s), jsNumber(callSite->columnNumber().zeroBasedInt())); + obj->putDirect(vm, JSC::Identifier::fromString(vm, "lineNumber"_s), line == OrdinalNumber::beforeFirst() ? JSC::jsNull() : jsNumber(line.oneBasedInt())); + obj->putDirect(vm, JSC::Identifier::fromString(vm, "columnNumber"_s), column == OrdinalNumber::beforeFirst() ? JSC::jsNull() : jsNumber(column.oneBasedInt())); obj->putDirect(vm, JSC::Identifier::fromString(vm, "functionName"_s), callSite->functionName()); return JSC::JSValue::encode(obj); } diff --git a/test/js/node/test/common/index.js b/test/js/node/test/common/index.js index 158d6eb7e46e..c5761cf8f176 100644 --- a/test/js/node/test/common/index.js +++ b/test/js/node/test/common/index.js @@ -641,7 +641,8 @@ function canCreateSymLink() { function getCallSite(top) { const originalStackFormatter = Error.prepareStackTrace; - Error.prepareStackTrace = (err, stack) => `${stack[0].getFileName()}:${stack[0].getLineNumber()}:${stack[0].getColumnNumber()}`; + Error.prepareStackTrace = (err, stack) => + `${stack[0].getFileName()}:${stack[0].getLineNumber()}`; const err = new Error(); Error.captureStackTrace(err, top); // With the V8 Error API, the stack is not formatted until it is accessed diff --git a/test/js/node/test/parallel/test-common-must-not-call.js b/test/js/node/test/parallel/test-common-must-not-call.js index 4b205be1934e..b3c94a2390ff 100644 --- a/test/js/node/test/parallel/test-common-must-not-call.js +++ b/test/js/node/test/parallel/test-common-must-not-call.js @@ -26,14 +26,14 @@ const createValidate = (line, args = []) => common.mustCall((e) => { assert.strictEqual(rest, line + argsInfo); }); -const validate1 = createValidate('9:29'); +const validate1 = createValidate('9'); try { testFunction1(); } catch (e) { validate1(e); } -const validate2 = createValidate('11:29', ['hello', 42]); +const validate2 = createValidate('11', ['hello', 42]); try { testFunction2('hello', 42); } catch (e) { diff --git a/test/js/node/v8/capture-stack-trace.test.js b/test/js/node/v8/capture-stack-trace.test.js index 6cd46a1ad90a..e957500f930c 100644 --- a/test/js/node/v8/capture-stack-trace.test.js +++ b/test/js/node/v8/capture-stack-trace.test.js @@ -551,6 +551,57 @@ test("CallFrame.p.isNative", () => { Error.prepareStackTrace = prevPrepareStackTrace; }); +// https://github.com/oven-sh/bun/issues/17303 +// https://github.com/oven-sh/bun/issues/18250 +test("CallFrame.p.getColumnNumber is 1-based and matches toString()", async () => { + // Run in a subprocess so the column isn't shifted by the CJS-wrapper transform applied + // to .js test files: the function-call column must be observable at exactly 1. + const src = [ + `let frames;`, + `Error.prepareStackTrace = (_, s) => s;`, + `function callee() { frames = new Error().stack; }`, + `callee();`, + `Error.prepareStackTrace = undefined;`, + `const callee0 = frames[0];`, + `const caller = frames[1];`, + `const colOf = f => Number(/:(\\d+):(\\d+)\\)?$/.exec(f.toString())[2]);`, + `console.log(JSON.stringify({`, + ` calleeMatch: callee0.getColumnNumber() === colOf(callee0),`, + ` callerCol: caller.getColumnNumber(),`, + ` callerStrCol: colOf(caller),`, + ` json: callee0.toJSON().columnNumber === callee0.getColumnNumber(),`, + `}));`, + ].join("\n"); + await using proc = Bun.spawn({ cmd: [bunExe(), "-e", src], env: bunEnv, stdout: "pipe", stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + // `callee()` starts at column 1; V8 reports 1, source-map-support subtracts 1 to get 0. + expect(JSON.parse(stdout)).toEqual({ calleeMatch: true, callerCol: 1, callerStrCol: 1, json: true }); + expect(exitCode).toBe(0); +}); + +test("CallFrame.p.getLineNumber/getColumnNumber return null for native frames", () => { + let prevPrepareStackTrace = Error.prepareStackTrace; + Error.prepareStackTrace = (e, s) => s; + let frames; + nativeFrameForTesting(() => { + const err = new Error(""); + Error.captureStackTrace(err); + frames = err.stack; + return 0; + }); + Error.prepareStackTrace = prevPrepareStackTrace; + const nativeFrame = frames[1]; + expect(nativeFrame.isNative()).toBe(true); + expect(nativeFrame.getLineNumber()).toBeNull(); + expect(nativeFrame.getColumnNumber()).toBeNull(); + const json = nativeFrame.toJSON(); + expect({ lineNumber: json.lineNumber, columnNumber: json.columnNumber }).toEqual({ + lineNumber: null, + columnNumber: null, + }); +}); + test("return non-strings from Error.prepareStackTrace", () => { // This behavior is allowed by V8 and used by the node-depd npm package. let prevPrepareStackTrace = Error.prepareStackTrace;