diff --git a/scripts/runner.node.mjs b/scripts/runner.node.mjs index 1bebf2e1b3..d3e108083a 100755 --- a/scripts/runner.node.mjs +++ b/scripts/runner.node.mjs @@ -161,10 +161,6 @@ const { values: options, positionals: filters } = parseArgs({ type: "boolean", default: false, }, - ["fail-on-coredump-or-report"]: { - type: "boolean", - default: false, // STAB-861 - }, }, }); @@ -1093,7 +1089,7 @@ async function spawnBun(execPath, { args, cwd, timeout, env, stdout, stderr }) { crashes += `main process killed by ${result.signalCode} but no core file found\n`; } - if (options["fail-on-coredump-or-report"] && newCores.length > 0) { + if (newCores.length > 0) { result.ok = false; if (!isAlwaysFailure(result.error)) result.error = "core dumped"; } @@ -1135,15 +1131,17 @@ async function spawnBun(execPath, { args, cwd, timeout, env, stdout, stderr }) { // When Bun crashes, it exits before the subcommand it runs to upload the crash report has necessarily finished. // So wait a little bit to make sure that the crash report has at least started uploading // (once the server sees the /ack request then /traces will wait for any crashes to finish processing) + // There is a bug that if a test causes crash reports but exits with code 0, the crash reports will instead + // be attributed to the next test that fails. I'm not sure how to fix this without adding a sleep in between + // all tests (which would slow down CI a lot). await setTimeoutPromise(500); const response = await fetch(`http://localhost:${remapPort}/traces`); if (!response.ok || response.status !== 200) throw new Error(`server responded with code ${response.status}`); const traces = await response.json(); if (traces.length > 0) { - if (options["fail-on-coredump-or-report"]) { - result.ok = false; - if (!isAlwaysFailure(result.error)) result.error = "crash reported"; - } + result.ok = false; + if (!isAlwaysFailure(result.error)) result.error = "crash reported"; + crashes += `${traces.length} crashes reported during this test\n`; for (const t of traces) { if (t.failed_parse) { @@ -1605,20 +1603,30 @@ async function getVendorTests(cwd) { const vendorPath = join(cwd, "vendor", name); if (!existsSync(vendorPath)) { - await spawnSafe({ + const { ok, error } = await spawnSafe({ command: "git", args: ["clone", "--depth", "1", "--single-branch", repository, vendorPath], timeout: testTimeout, cwd, }); + if (!ok) throw new Error(`failed to git clone vendor '${name}': ${error}`); } - await spawnSafe({ + let { ok, error } = await spawnSafe({ command: "git", args: ["fetch", "--depth", "1", "origin", "tag", tag], timeout: testTimeout, cwd: vendorPath, }); + if (!ok) throw new Error(`failed to fetch tag ${tag} for vendor '${name}': ${error}`); + + ({ ok, error } = await spawnSafe({ + command: "git", + args: ["checkout", tag], + timeout: testTimeout, + cwd: vendorPath, + })); + if (!ok) throw new Error(`failed to checkout tag ${tag} for vendor '${name}': ${error}`); const packageJsonPath = join(vendorPath, "package.json"); if (!existsSync(packageJsonPath)) { diff --git a/src/bun.js/bindings/BunProcess.cpp b/src/bun.js/bindings/BunProcess.cpp index 177b21fe28..936935529a 100644 --- a/src/bun.js/bindings/BunProcess.cpp +++ b/src/bun.js/bindings/BunProcess.cpp @@ -152,6 +152,8 @@ BUN_DECLARE_HOST_FUNCTION(Bun__Process__send); extern "C" void Process__emitDisconnectEvent(Zig::GlobalObject* global); extern "C" void Process__emitErrorEvent(Zig::GlobalObject* global, EncodedJSValue value); +extern "C" void Bun__suppressCrashOnProcessKillSelfIfDesired(); + static Process* getProcessObject(JSC::JSGlobalObject* lexicalGlobalObject, JSValue thisValue); bool setProcessExitCodeInner(JSC::JSGlobalObject* lexicalGlobalObject, Process* process, JSValue code); @@ -3655,6 +3657,9 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionReallyKill, (JSC::JSGlobalObject * glob RETURN_IF_EXCEPTION(scope, {}); #if !OS(WINDOWS) + if (pid == getpid()) { + Bun__suppressCrashOnProcessKillSelfIfDesired(); + } int result = kill(pid, signal); if (result < 0) result = errno; diff --git a/src/bun.js/bindings/napi.h b/src/bun.js/bindings/napi.h index 8358142654..9cbf4ac3a7 100644 --- a/src/bun.js/bindings/napi.h +++ b/src/bun.js/bindings/napi.h @@ -20,7 +20,7 @@ #include extern "C" void napi_internal_register_cleanup_zig(napi_env env); -extern "C" void napi_internal_crash_in_gc(napi_env); +extern "C" void napi_internal_suppress_crash_on_abort_if_desired(); extern "C" void Bun__crashHandler(const char* message, size_t message_len); namespace Napi { @@ -47,7 +47,11 @@ struct napi_async_cleanup_hook_handle__ { } }; -#define NAPI_ABORT(message) Bun__crashHandler(message "", sizeof(message "") - 1) +#define NAPI_ABORT(message) \ + do { \ + napi_internal_suppress_crash_on_abort_if_desired(); \ + Bun__crashHandler(message "", sizeof(message "") - 1); \ + } while (0) #define NAPI_PERISH(...) \ do { \ diff --git a/src/bun.js/node/node_process.zig b/src/bun.js/node/node_process.zig index 5231e8201d..63f6e087d2 100644 --- a/src/bun.js/node/node_process.zig +++ b/src/bun.js/node/node_process.zig @@ -317,10 +317,16 @@ comptime { } } -pub export fn Bun__NODE_NO_WARNINGS() callconv(.C) bool { +pub export fn Bun__NODE_NO_WARNINGS() bool { return bun.getRuntimeFeatureFlag(.NODE_NO_WARNINGS); } +pub export fn Bun__suppressCrashOnProcessKillSelfIfDesired() void { + if (bun.getRuntimeFeatureFlag(.BUN_INTERNAL_SUPPRESS_CRASH_ON_PROCESS_KILL_SELF)) { + bun.crash_handler.suppressReporting(); + } +} + pub export const Bun__version: [*:0]const u8 = "v" ++ bun.Global.package_json_version; pub export const Bun__version_with_sha: [*:0]const u8 = "v" ++ bun.Global.package_json_version_with_sha; pub export const Bun__versions_boringssl: [*:0]const u8 = bun.Global.versions.boringssl; diff --git a/src/crash_handler.zig b/src/crash_handler.zig index 45eda9a025..3755b676a2 100644 --- a/src/crash_handler.zig +++ b/src/crash_handler.zig @@ -57,6 +57,11 @@ var before_crash_handlers: std.ArrayListUnmanaged(struct { *anyopaque, *const On var before_crash_handlers_mutex: bun.Mutex = .{}; +/// Prevents crash reports from being uploaded to any server. Reports will still be printed and +/// abort the process. Overrides BUN_CRASH_REPORT_URL, BUN_ENABLE_CRASH_REPORTING, and all other +/// things that affect crash reporting. See suppressReporting() for intended usage. +var suppress_reporting: bool = false; + /// This structure and formatter must be kept in sync with `bun.report`'s decoder implementation. pub const CrashReason = union(enum) { /// From @panic() @@ -1350,6 +1355,8 @@ fn writeU64AsTwoVLQs(writer: anytype, addr: usize) !void { } fn isReportingEnabled() bool { + if (suppress_reporting) return false; + // If trying to test the crash handler backend, implicitly enable reporting if (bun.getenvZ("BUN_CRASH_REPORT_URL")) |value| { return value.len > 0; @@ -1703,6 +1710,29 @@ pub fn dumpCurrentStackTrace(first_address: ?usize, limits: WriteStackTraceLimit dumpStackTrace(stack, limits); } +/// If POSIX, and the existing soft limit for core dumps (ulimit -Sc) is nonzero, change it to zero. +/// Used in places where we intentionally crash for testing purposes so that we don't clutter CI +/// with core dumps. +fn suppressCoreDumpsIfNecessary() void { + if (bun.Environment.isPosix) { + var existing_limit = std.posix.getrlimit(.CORE) catch return; + if (existing_limit.cur > 0 or existing_limit.cur == std.posix.RLIM.INFINITY) { + existing_limit.cur = 0; + std.posix.setrlimit(.CORE, existing_limit) catch {}; + } + } +} + +/// From now on, prevent crashes from being reported to bun.report or the URL overridden in +/// BUN_CRASH_REPORT_URL. Should only be used for tests that are going to intentionally crash, +/// so that they do not fail CI due to having a crash reported. And those cases should guard behind +/// a feature flag and call right before the crash, in order to make sure that crashes other than +/// the expected one are not suppressed. +pub fn suppressReporting() void { + suppressCoreDumpsIfNecessary(); + suppress_reporting = true; +} + /// A variant of `std.builtin.StackTrace` that stores its data within itself /// instead of being a pointer. This allows storing captured stack traces /// for later printing. @@ -1787,6 +1817,7 @@ pub const js_bindings = struct { pub fn jsSegfault(_: *jsc.JSGlobalObject, _: *jsc.CallFrame) bun.JSError!jsc.JSValue { @setRuntimeSafety(false); + suppressCoreDumpsIfNecessary(); const ptr: [*]align(1) u64 = @ptrFromInt(0xDEADBEEF); ptr[0] = 0xDEADBEEF; std.mem.doNotOptimizeAway(&ptr); @@ -1794,6 +1825,7 @@ pub const js_bindings = struct { } pub fn jsPanic(_: *jsc.JSGlobalObject, _: *jsc.CallFrame) bun.JSError!jsc.JSValue { + suppressCoreDumpsIfNecessary(); bun.crash_handler.panicImpl("invoked crashByPanic() handler", null, null); } @@ -1802,10 +1834,12 @@ pub const js_bindings = struct { } pub fn jsOutOfMemory(_: *jsc.JSGlobalObject, _: *jsc.CallFrame) bun.JSError!jsc.JSValue { + suppressCoreDumpsIfNecessary(); bun.outOfMemory(); } pub fn jsRaiseIgnoringPanicHandler(_: *jsc.JSGlobalObject, _: *jsc.CallFrame) bun.JSError!jsc.JSValue { + suppressCoreDumpsIfNecessary(); bun.Global.raiseIgnoringPanicHandler(.SIGSEGV); } @@ -2166,6 +2200,9 @@ export fn CrashHandler__setInsideNativePlugin(name: ?[*:0]const u8) callconv(.C) export fn CrashHandler__unsupportedUVFunction(name: ?[*:0]const u8) callconv(.C) void { bun.analytics.Features.unsupported_uv_function += 1; unsupported_uv_function = name; + if (bun.getRuntimeFeatureFlag(.BUN_INTERNAL_SUPPRESS_CRASH_ON_UV_STUB)) { + suppressReporting(); + } std.debug.panic("unsupported uv function: {s}", .{name.?}); } diff --git a/src/feature_flags.zig b/src/feature_flags.zig index 089ba43a0d..7e0cf90912 100644 --- a/src/feature_flags.zig +++ b/src/feature_flags.zig @@ -31,6 +31,12 @@ pub const RuntimeFeatureFlag = enum { BUN_FEATURE_FLAG_NO_LIBDEFLATE, BUN_INSTRUMENTS, BUN_INTERNAL_BUNX_INSTALL, + /// Suppress crash reporting and creating a core dump when we abort due to an unsupported libuv function being called + BUN_INTERNAL_SUPPRESS_CRASH_ON_UV_STUB, + /// Suppress crash reporting and creating a core dump when we abort due to a fatal Node-API error + BUN_INTERNAL_SUPPRESS_CRASH_ON_NAPI_ABORT, + /// Suppress crash reporting and creating a core dump when `process._kill()` is passed its own PID + BUN_INTERNAL_SUPPRESS_CRASH_ON_PROCESS_KILL_SELF, BUN_NO_CODESIGN_MACHO_BINARY, BUN_TRACE, NODE_NO_WARNINGS, diff --git a/src/napi/napi.zig b/src/napi/napi.zig index d7cca564a7..2181cc94dd 100644 --- a/src/napi/napi.zig +++ b/src/napi/napi.zig @@ -1155,6 +1155,7 @@ fn napiSpan(ptr: anytype, len: usize) []const u8 { } pub export fn napi_fatal_error(location_ptr: ?[*:0]const u8, location_len: usize, message_ptr: ?[*:0]const u8, message_len_: usize) noreturn { log("napi_fatal_error", .{}); + napi_internal_suppress_crash_on_abort_if_desired(); var message = napiSpan(message_ptr, message_len_); if (message.len == 0) { message = "fatal error"; @@ -1329,6 +1330,12 @@ pub export fn napi_internal_register_cleanup_zig(env_: napi_env) void { }.callback); } +pub export fn napi_internal_suppress_crash_on_abort_if_desired() void { + if (bun.getRuntimeFeatureFlag(.BUN_INTERNAL_SUPPRESS_CRASH_ON_NAPI_ABORT)) { + bun.crash_handler.suppressReporting(); + } +} + extern fn napi_internal_remove_finalizer(env: napi_env, fun: napi_finalize, hint: ?*anyopaque, data: ?*anyopaque) callconv(.C) void; pub const Finalizer = struct { diff --git a/src/string/immutable.zig b/src/string/immutable.zig index 2a5804955f..dcb79a9174 100644 --- a/src/string/immutable.zig +++ b/src/string/immutable.zig @@ -1613,34 +1613,34 @@ pub fn indexOfLineRanges(text: []const u8, target_line: u32, comptime line_range cursor.i = current_i; cursor.width = 0; const current_line_range: LineRange = brk: { - if (iter.next(&cursor)) { - const codepoint = cursor.c; - switch (codepoint) { - '\n' => { - const start = prev_end; - prev_end = cursor.i; + bun.assert(iter.next(&cursor)); // cursor points to current_i where we know there is some character + const codepoint = cursor.c; + switch (codepoint) { + '\n' => { + const start = prev_end; + prev_end = cursor.i; + break :brk .{ + .start = start, + .end = cursor.i + 1, + }; + }, + '\r' => { + const current_end = cursor.i; + if (iter.next(&cursor) and cursor.c == '\n') { + defer prev_end = cursor.i; break :brk .{ - .start = start, + .start = prev_end, + .end = current_end, + }; + } else { + break :brk .{ + .start = prev_end, .end = cursor.i + 1, }; - }, - '\r' => { - const current_end = cursor.i; - if (iter.next(&cursor)) { - const codepoint2 = cursor.c; - if (codepoint2 == '\n') { - defer prev_end = cursor.i; - break :brk .{ - .start = prev_end, - .end = current_end, - }; - } - } - }, - else => continue, - } + } + }, + else => continue, } - @panic("unreachable"); }; if (ranges.len == line_range_count and current_line <= target_line) { diff --git a/test/cli/run/fixture-crash.js b/test/cli/run/fixture-crash.js index c90049da5d..7e9082c78d 100644 --- a/test/cli/run/fixture-crash.js +++ b/test/cli/run/fixture-crash.js @@ -3,7 +3,7 @@ try { crash_handler = require("bun:internal-for-testing").crash_handler; } catch { console.error("This version of bun does not have internal-for-testing exposed"); - console.error("BUN_GARBAGE_COLLECTOR_LEVEL=0 BUN_FEATURE_FLAG_INTERNALS_FOR_TESTING=1 bun"); + console.error("BUN_GARBAGE_COLLECTOR_LEVEL=0 BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING=1 bun"); process.exit(1); } diff --git a/test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts b/test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts index ae2276109c..9d9d027d08 100644 --- a/test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts +++ b/test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts @@ -203,7 +203,7 @@ class SnapshotTester { if (!opts.shouldNotError) { if (!isFirst) { // make sure it fails first: - expect((await $`cd ${this.dir} && ${bunExe()} test ./snapshot.test.ts`.nothrow().quiet()).exitCode).not.toBe(0); + expect((await $`cd ${this.dir} && ${bunExe()} test ./snapshot.test.ts`.nothrow().quiet()).exitCode).toBe(1); // make sure the existing snapshot is unchanged: expect(await this.getSnapshotContents()).toBe(this.targetSnapshotContents); } @@ -395,7 +395,7 @@ class InlineSnapshotTester { stdio: ["pipe", "pipe", "pipe"], }); expect(spawnres.stderr.toString()).toInclude(eopts.msg); - expect(spawnres.exitCode).not.toBe(0); + expect(spawnres.exitCode).toBe(1); expect(this.readfile(thefile)).toEqual(code); } test(cb: (v: (a: string, b: string, c: string) => string) => string): void { @@ -429,7 +429,7 @@ class InlineSnapshotTester { stdio: ["pipe", "pipe", "pipe"], }); expect(spawnres.stderr.toString()).toInclude("error:"); - expect(spawnres.exitCode).not.toBe(0); + expect(spawnres.exitCode).toBe(1); expect(this.readfile(thefile)).toEqual(before_value); } diff --git a/test/js/node/child_process/child_process.test.ts b/test/js/node/child_process/child_process.test.ts index 09ac691b4c..79a9e76c94 100644 --- a/test/js/node/child_process/child_process.test.ts +++ b/test/js/node/child_process/child_process.test.ts @@ -454,7 +454,10 @@ it.if(!isWindows)("spawnSync correctly reports signal codes", () => { process.kill(process.pid, "SIGTRAP"); `; - const { signal } = spawnSync(bunExe(), ["-e", trapCode]); + const { signal } = spawnSync(bunExe(), ["-e", trapCode], { + // @ts-expect-error + env: { ...bunEnv, BUN_INTERNAL_SUPPRESS_CRASH_ON_PROCESS_KILL_SELF: "1" }, + }); expect(signal).toBe("SIGTRAP"); }); diff --git a/test/napi/node-napi.test.ts b/test/napi/node-napi.test.ts index 7311247646..8b415a1a28 100644 --- a/test/napi/node-napi.test.ts +++ b/test/napi/node-napi.test.ts @@ -74,6 +74,10 @@ if (isMusl) { failingJsNativeApiTests = jsNativeApiTests; } +// Tests that intentionally abort and should not generate core dumps when they abort +// due to a Node-API error +const abortingJsNativeApiTests = ["test_finalizer/test_fatal_finalize.js"]; + for (const t of failingJsNativeApiTests) { if (!jsNativeApiTests.includes(t)) { console.error(`attempt to skip ${t} which is not a real js-native-api test`); @@ -129,13 +133,16 @@ describe("js-native-api tests", () => { for (const test of jsNativeApiTests) { describe.skipIf(failingJsNativeApiTests.includes(test))(`${test}`, () => { it("passes", () => { + const env = abortingJsNativeApiTests.includes(test) + ? { ...bunEnv, BUN_INTERNAL_SUPPRESS_CRASH_ON_NAPI_ABORT: "1" } + : bunEnv; const result = spawnSync({ cmd: [bunExe(), "run", test], cwd: jsNativeApiRoot, stderr: "inherit", stdout: "ignore", stdin: "inherit", - env: bunEnv, + env, }); expect(result.success).toBeTrue(); expect(result.exitCode).toBe(0); diff --git a/test/napi/uv_stub.test.ts b/test/napi/uv_stub.test.ts index 068617e0fc..e92e22af75 100644 --- a/test/napi/uv_stub.test.ts +++ b/test/napi/uv_stub.test.ts @@ -76,8 +76,10 @@ describe.if(!isWindows)("uv stubs", () => { for (const symbol of symbols_to_test) { test(`should crash when calling unsupported uv functions: ${symbol}`, async () => { - console.log("GO:", symbol); - const { stderr } = await Bun.$`${bunExe()} run index.ts ${symbol}`.cwd(tempdir).throws(false).quiet(); + const { stderr } = await Bun.$`BUN_INTERNAL_SUPPRESS_CRASH_ON_UV_STUB=1 ${bunExe()} run index.ts ${symbol}` + .cwd(tempdir) + .throws(false) + .quiet(); const stderrStr = stderr.toString(); expect(stderrStr).toContain("Bun encountered a crash when running a NAPI module that tried to call"); expect(stderrStr).toContain(symbol); diff --git a/test/no-validate-exceptions.txt b/test/no-validate-exceptions.txt index 2c968e852b..0e166286a7 100644 --- a/test/no-validate-exceptions.txt +++ b/test/no-validate-exceptions.txt @@ -52,6 +52,7 @@ vendor/elysia/test/macro/macro.test.ts vendor/elysia/test/path/group.test.ts vendor/elysia/test/path/guard.test.ts vendor/elysia/test/path/path.test.ts +vendor/elysia/test/path/plugin.test.ts vendor/elysia/test/plugins/affix.test.ts vendor/elysia/test/plugins/checksum.test.ts vendor/elysia/test/plugins/error-propagation.test.ts