From 13723f267eb87f8c28ef0769cdf7e233b971326e Mon Sep 17 00:00:00 2001 From: Luke Edwards Date: Wed, 29 May 2024 16:16:27 -0700 Subject: [PATCH] feat: Add `Deno.exitCode` API (#23609) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commits adds the ability to set a would-be exit code for the Deno process without forcing an immediate exit, through the new `Deno.exitCode` API. - **Implements `Deno.exitCode` getter and setter**: Adds support for setting and retrieving a would-be exit code via `Deno.exitCode`. This allows for asynchronous cleanup before process termination without immediately exiting. - **Ensures type safety**: The setter for `Deno.exitCode` validates that the provided value is a number, throwing a TypeError if not, to ensure that only valid exit codes are set. Closes to #23605 --------- Co-authored-by: Bartek Iwańczuk --- cli/js/40_test.js | 19 ++++- cli/tests/unit/os_test.ts | 94 ++++++++++++++++++++++ cli/tsc/dts/lib.deno.ns.d.ts | 17 ++++ ext/node/polyfills/process.ts | 63 +++++++++++---- runtime/js/30_os.js | 21 ++++- runtime/js/99_main.js | 8 ++ runtime/ops/os/mod.rs | 10 ++- tests/specs/run/exit_code/__test__.jsonc | 5 ++ tests/specs/run/exit_code/main.js | 7 ++ tests/specs/run/exit_code/main.out | 1 + tests/specs/test/exit_code/__test__.jsonc | 5 ++ tests/specs/test/exit_code/main.js | 3 + tests/specs/test/exit_code/main.out | 17 ++++ tests/specs/test/exit_code2/__test__.jsonc | 5 ++ tests/specs/test/exit_code2/main.js | 7 ++ tests/specs/test/exit_code2/main.out | 25 ++++++ tests/specs/test/exit_code3/__test__.jsonc | 5 ++ tests/specs/test/exit_code3/main.js | 6 ++ tests/specs/test/exit_code3/main.out | 18 +++++ tests/unit_node/process_test.ts | 10 +-- 20 files changed, 322 insertions(+), 24 deletions(-) create mode 100644 cli/tests/unit/os_test.ts create mode 100644 tests/specs/run/exit_code/__test__.jsonc create mode 100644 tests/specs/run/exit_code/main.js create mode 100644 tests/specs/run/exit_code/main.out create mode 100644 tests/specs/test/exit_code/__test__.jsonc create mode 100644 tests/specs/test/exit_code/main.js create mode 100644 tests/specs/test/exit_code/main.out create mode 100644 tests/specs/test/exit_code2/__test__.jsonc create mode 100644 tests/specs/test/exit_code2/main.js create mode 100644 tests/specs/test/exit_code2/main.out create mode 100644 tests/specs/test/exit_code3/__test__.jsonc create mode 100644 tests/specs/test/exit_code3/main.js create mode 100644 tests/specs/test/exit_code3/main.out diff --git a/cli/js/40_test.js b/cli/js/40_test.js index 2877bfa9b5..5a081e2175 100644 --- a/cli/js/40_test.js +++ b/cli/js/40_test.js @@ -28,6 +28,10 @@ const { import { setExitHandler } from "ext:runtime/30_os.js"; +// Capture `Deno` global so that users deleting or mangling it, won't +// have impact on our sanitizers. +const DenoNs = globalThis.Deno; + /** * @typedef {{ * id: number, @@ -101,7 +105,20 @@ function assertExit(fn, isTest) { try { const innerResult = await fn(...new SafeArrayIterator(params)); - if (innerResult) return innerResult; + const exitCode = DenoNs.exitCode; + if (exitCode !== 0) { + // Reset the code to allow other tests to run... + DenoNs.exitCode = 0; + // ...and fail the current test. + throw new Error( + `${ + isTest ? "Test case" : "Bench" + } finished with exit code set to ${exitCode}.`, + ); + } + if (innerResult) { + return innerResult; + } } finally { setExitHandler(null); } diff --git a/cli/tests/unit/os_test.ts b/cli/tests/unit/os_test.ts new file mode 100644 index 0000000000..af6ef219a2 --- /dev/null +++ b/cli/tests/unit/os_test.ts @@ -0,0 +1,94 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +import { assertEquals, assertThrows } from "../../testing/asserts.ts"; + +Deno.test("Deno.exitCode getter and setter", () => { + // Initial value is 0 + assertEquals(Deno.exitCode, 0); + + // Set a new value + Deno.exitCode = 5; + assertEquals(Deno.exitCode, 5); + + // Reset to initial value + Deno.exitCode = 0; + assertEquals(Deno.exitCode, 0); +}); + +Deno.test("Setting Deno.exitCode to NaN throws TypeError", () => { + // @ts-expect-error; + Deno.exitCode = "123"; + assertEquals(Deno.exitCode, 123); + + // Reset + Deno.exitCode = 0; + assertEquals(Deno.exitCode, 0); + + // Throws on non-number values + assertThrows( + () => { + // @ts-expect-error Testing for runtime error + Deno.exitCode = "not a number"; + }, + TypeError, + "Exit code must be a number.", + ); +}); + +Deno.test("Setting Deno.exitCode does not cause an immediate exit", () => { + let exited = false; + const originalExit = Deno.exit; + + // @ts-expect-error; read-only + Deno.exit = () => { + exited = true; + }; + + Deno.exitCode = 1; + assertEquals(exited, false); + + // @ts-expect-error; read-only + Deno.exit = originalExit; +}); + +Deno.test("Running Deno.exit(value) overrides Deno.exitCode", () => { + let args: unknown[] | undefined; + + const originalExit = Deno.exit; + // @ts-expect-error; read-only + Deno.exit = (...x) => { + args = x; + }; + + Deno.exitCode = 42; + Deno.exit(0); + + assertEquals(args, [0]); + // @ts-expect-error; read-only + Deno.exit = originalExit; +}); + +Deno.test("Running Deno.exit() uses Deno.exitCode as fallback", () => { + let args: unknown[] | undefined; + + const originalExit = Deno.exit; + // @ts-expect-error; read-only + Deno.exit = (...x) => { + args = x; + }; + + Deno.exitCode = 42; + Deno.exit(); + + assertEquals(args, [42]); + // @ts-expect-error; read-only + Deno.exit = originalExit; +}); + +Deno.test("Retrieving the set exit code before process termination", () => { + Deno.exitCode = 42; + assertEquals(Deno.exitCode, 42); + + // Reset to initial value + Deno.exitCode = 0; +}); diff --git a/cli/tsc/dts/lib.deno.ns.d.ts b/cli/tsc/dts/lib.deno.ns.d.ts index 76b59761c4..cf8e4ba053 100644 --- a/cli/tsc/dts/lib.deno.ns.d.ts +++ b/cli/tsc/dts/lib.deno.ns.d.ts @@ -1466,6 +1466,23 @@ declare namespace Deno { */ export function exit(code?: number): never; + /** The exit code for the Deno process. + * + * If no exit code has been supplied, then Deno will assume a return code of `0`. + * + * When setting an exit code value, a number or non-NaN string must be provided, + * otherwise a TypeError will be thrown. + * + * ```ts + * console.log(Deno.exitCode); //-> 0 + * Deno.exitCode = 1; + * console.log(Deno.exitCode); //-> 1 + * ``` + * + * @category Runtime + */ + export var exitCode: number; + /** An interface containing methods to interact with the process environment * variables. * diff --git a/ext/node/polyfills/process.ts b/ext/node/polyfills/process.ts index f742e6634a..9a28137af9 100644 --- a/ext/node/polyfills/process.ts +++ b/ext/node/polyfills/process.ts @@ -10,7 +10,6 @@ import { op_geteuid, op_node_process_kill, op_process_abort, - op_set_exit_code, } from "ext:core/ops"; import { warnNotImplemented } from "ext:deno_node/_utils.ts"; @@ -49,6 +48,7 @@ import { } from "ext:deno_node/_next_tick.ts"; import { isWindows } from "ext:deno_node/_util/os.ts"; import * as io from "ext:deno_io/12_io.js"; +import * as denoOs from "ext:runtime/30_os.js"; export let argv0 = ""; @@ -74,28 +74,31 @@ const notImplementedEvents = [ ]; export const argv: string[] = ["", ""]; -let globalProcessExitCode: number | undefined = undefined; + +// In Node, `process.exitCode` is initially `undefined` until set. +// And retains any value as long as it's nullish or number-ish. +let ProcessExitCode: undefined | null | string | number; /** https://nodejs.org/api/process.html#process_process_exit_code */ export const exit = (code?: number | string) => { if (code || code === 0) { - if (typeof code === "string") { - const parsedCode = parseInt(code); - globalProcessExitCode = isNaN(parsedCode) ? undefined : parsedCode; - } else { - globalProcessExitCode = code; - } + denoOs.setExitCode(code); + } else if (Number.isNaN(code)) { + denoOs.setExitCode(1); } + ProcessExitCode = denoOs.getExitCode(); if (!process._exiting) { process._exiting = true; // FIXME(bartlomieju): this is wrong, we won't be using syscall to exit // and thus the `unload` event will not be emitted to properly trigger "emit" // event on `process`. - process.emit("exit", process.exitCode || 0); + process.emit("exit", ProcessExitCode); } - process.reallyExit(process.exitCode || 0); + // Any valid thing `process.exitCode` set is already held in Deno.exitCode. + // At this point, we don't have to pass around Node's raw/string exit value. + process.reallyExit(ProcessExitCode); }; /** https://nodejs.org/api/process.html#processumaskmask */ @@ -433,14 +436,42 @@ Process.prototype._exiting = _exiting; /** https://nodejs.org/api/process.html#processexitcode_1 */ Object.defineProperty(Process.prototype, "exitCode", { get() { - return globalProcessExitCode; + return ProcessExitCode; }, - set(code: number | undefined) { - globalProcessExitCode = code; - code = parseInt(code) || 0; - if (!isNaN(code)) { - op_set_exit_code(code); + set(code: number | string | null | undefined) { + let parsedCode; + + if (typeof code === "number") { + if (Number.isNaN(code)) { + parsedCode = 1; + denoOs.setExitCode(parsedCode); + ProcessExitCode = parsedCode; + return; + } + + // This is looser than `denoOs.setExitCode` which requires exit code + // to be decimal or string of a decimal, but Node accept eg. 0x10. + parsedCode = parseInt(code); + denoOs.setExitCode(parsedCode); + ProcessExitCode = parsedCode; + return; } + + if (typeof code === "string") { + parsedCode = parseInt(code); + if (Number.isNaN(parsedCode)) { + throw new TypeError( + `The "code" argument must be of type number. Received type ${typeof code} (${code})`, + ); + } + denoOs.setExitCode(parsedCode); + ProcessExitCode = parsedCode; + return; + } + + // TODO(bartlomieju): hope for the best here. This should be further tightened. + denoOs.setExitCode(code); + ProcessExitCode = code; }, }); diff --git a/runtime/js/30_os.js b/runtime/js/30_os.js index 8948cf1ad0..866fad2878 100644 --- a/runtime/js/30_os.js +++ b/runtime/js/30_os.js @@ -7,6 +7,7 @@ import { op_exec_path, op_exit, op_get_env, + op_get_exit_code, op_gid, op_hostname, op_loadavg, @@ -21,7 +22,9 @@ import { const { Error, FunctionPrototypeBind, + NumberParseInt, SymbolFor, + TypeError, } = primordials; import { Event, EventTarget } from "ext:deno_web/02_event.js"; @@ -75,7 +78,7 @@ function exit(code) { if (typeof code === "number") { op_set_exit_code(code); } else { - code = 0; + code = op_get_exit_code(); } // Dispatches `unload` only when it's not dispatched yet. @@ -94,6 +97,20 @@ function exit(code) { throw new Error("Code not reachable"); } +function getExitCode() { + return op_get_exit_code(); +} + +function setExitCode(value) { + const code = NumberParseInt(value, 10); + if (typeof code !== "number") { + throw new TypeError( + `Exit code must be a number, got: ${code} (${typeof code}).`, + ); + } + op_set_exit_code(code); +} + function setEnv(key, value) { op_set_env(key, value); } @@ -126,12 +143,14 @@ export { env, execPath, exit, + getExitCode, gid, hostname, loadavg, networkInterfaces, osRelease, osUptime, + setExitCode, setExitHandler, systemMemoryInfo, uid, diff --git a/runtime/js/99_main.js b/runtime/js/99_main.js index 3fa9fc41b6..4f949b2146 100644 --- a/runtime/js/99_main.js +++ b/runtime/js/99_main.js @@ -674,6 +674,14 @@ ObjectDefineProperties(finalDenoNs, { return internals.future ? undefined : customInspect; }, }, + exitCode: { + get() { + return os.getExitCode(); + }, + set(value) { + os.setExitCode(value); + }, + }, }); const { diff --git a/runtime/ops/os/mod.rs b/runtime/ops/os/mod.rs index 7a71590baa..f6f55f68fa 100644 --- a/runtime/ops/os/mod.rs +++ b/runtime/ops/os/mod.rs @@ -32,6 +32,7 @@ deno_core::extension!( op_os_uptime, op_set_env, op_set_exit_code, + op_get_exit_code, op_system_memory_info, op_uid, op_runtime_memory_usage, @@ -60,12 +61,13 @@ deno_core::extension!( op_os_uptime, op_set_env, op_set_exit_code, + op_get_exit_code, op_system_memory_info, op_uid, op_runtime_memory_usage, ], middleware = |op| match op.name { - "op_exit" | "op_set_exit_code" => + "op_exit" | "op_set_exit_code" | "op_get_exit_code" => op.with_implementation_from(&deno_core::op_void_sync()), _ => op, }, @@ -164,6 +166,12 @@ fn op_set_exit_code(state: &mut OpState, #[smi] code: i32) { state.borrow_mut::().set(code); } +#[op2(fast)] +#[smi] +fn op_get_exit_code(state: &mut OpState) -> i32 { + state.borrow_mut::().get() +} + #[op2(fast)] fn op_exit(state: &mut OpState) { let code = state.borrow::().get(); diff --git a/tests/specs/run/exit_code/__test__.jsonc b/tests/specs/run/exit_code/__test__.jsonc new file mode 100644 index 0000000000..977f8b0e37 --- /dev/null +++ b/tests/specs/run/exit_code/__test__.jsonc @@ -0,0 +1,5 @@ +{ + "args": "run main.js", + "exitCode": 42, + "output": "main.out" +} diff --git a/tests/specs/run/exit_code/main.js b/tests/specs/run/exit_code/main.js new file mode 100644 index 0000000000..1ba1f06c4f --- /dev/null +++ b/tests/specs/run/exit_code/main.js @@ -0,0 +1,7 @@ +if (Deno.exitCode != 0) { + throw new Error("boom!"); +} + +Deno.exitCode = 42; + +console.log("Deno.exitCode", Deno.exitCode); diff --git a/tests/specs/run/exit_code/main.out b/tests/specs/run/exit_code/main.out new file mode 100644 index 0000000000..25f2de2d5a --- /dev/null +++ b/tests/specs/run/exit_code/main.out @@ -0,0 +1 @@ +Deno.exitCode 42 diff --git a/tests/specs/test/exit_code/__test__.jsonc b/tests/specs/test/exit_code/__test__.jsonc new file mode 100644 index 0000000000..37d678453b --- /dev/null +++ b/tests/specs/test/exit_code/__test__.jsonc @@ -0,0 +1,5 @@ +{ + "args": "test main.js", + "exitCode": 1, + "output": "main.out" +} diff --git a/tests/specs/test/exit_code/main.js b/tests/specs/test/exit_code/main.js new file mode 100644 index 0000000000..768bb4165e --- /dev/null +++ b/tests/specs/test/exit_code/main.js @@ -0,0 +1,3 @@ +Deno.test("Deno.exitCode", () => { + Deno.exitCode = 42; +}); diff --git a/tests/specs/test/exit_code/main.out b/tests/specs/test/exit_code/main.out new file mode 100644 index 0000000000..2562695a02 --- /dev/null +++ b/tests/specs/test/exit_code/main.out @@ -0,0 +1,17 @@ +running 1 test from ./main.js +Deno.exitCode ... FAILED ([WILDCARD]) + + ERRORS + +Deno.exitCode => ./main.js:1:6 +error: Error: Test case finished with exit code set to 42. + at exitSanitizer (ext:cli/40_test.js:113:15) + at async outerWrapped (ext:cli/40_test.js:134:14) + + FAILURES + +Deno.exitCode => ./main.js:1:6 + +FAILED | 0 passed | 1 failed ([WILDCARD]) + +error: Test failed diff --git a/tests/specs/test/exit_code2/__test__.jsonc b/tests/specs/test/exit_code2/__test__.jsonc new file mode 100644 index 0000000000..37d678453b --- /dev/null +++ b/tests/specs/test/exit_code2/__test__.jsonc @@ -0,0 +1,5 @@ +{ + "args": "test main.js", + "exitCode": 1, + "output": "main.out" +} diff --git a/tests/specs/test/exit_code2/main.js b/tests/specs/test/exit_code2/main.js new file mode 100644 index 0000000000..2e6398a580 --- /dev/null +++ b/tests/specs/test/exit_code2/main.js @@ -0,0 +1,7 @@ +Deno.test("Deno.exitCode", () => { + Deno.exitCode = 5; + throw new Error(""); +}); + +Deno.test("success", () => { +}); diff --git a/tests/specs/test/exit_code2/main.out b/tests/specs/test/exit_code2/main.out new file mode 100644 index 0000000000..adc9cb5775 --- /dev/null +++ b/tests/specs/test/exit_code2/main.out @@ -0,0 +1,25 @@ +running 2 tests from ./main.js +Deno.exitCode ... FAILED ([WILDCARD]) +success ... FAILED ([WILDCARD]) + + ERRORS + +Deno.exitCode => ./main.js:1:6 +error: Error + throw new Error(""); + ^ + at [WILDCARD]/exit_code2/main.js:3:9 + +success => ./main.js:6:6 +error: Error: Test case finished with exit code set to 5. + at exitSanitizer (ext:cli/40_test.js:113:15) + at async outerWrapped (ext:cli/40_test.js:134:14) + + FAILURES + +Deno.exitCode => ./main.js:1:6 +success => ./main.js:6:6 + +FAILED | 0 passed | 2 failed ([WILDCARD]) + +error: Test failed diff --git a/tests/specs/test/exit_code3/__test__.jsonc b/tests/specs/test/exit_code3/__test__.jsonc new file mode 100644 index 0000000000..37d678453b --- /dev/null +++ b/tests/specs/test/exit_code3/__test__.jsonc @@ -0,0 +1,5 @@ +{ + "args": "test main.js", + "exitCode": 1, + "output": "main.out" +} diff --git a/tests/specs/test/exit_code3/main.js b/tests/specs/test/exit_code3/main.js new file mode 100644 index 0000000000..a859afbaef --- /dev/null +++ b/tests/specs/test/exit_code3/main.js @@ -0,0 +1,6 @@ +Deno.test("Deno.exitCode", () => { + Deno.exitCode = 42; +}); + +Deno.test("success", () => { +}); diff --git a/tests/specs/test/exit_code3/main.out b/tests/specs/test/exit_code3/main.out new file mode 100644 index 0000000000..6e333bf425 --- /dev/null +++ b/tests/specs/test/exit_code3/main.out @@ -0,0 +1,18 @@ +running 2 tests from ./main.js +Deno.exitCode ... FAILED ([WILDCARD]) +success ... ok ([WILDCARD]) + + ERRORS + +Deno.exitCode => ./main.js:1:6 +error: Error: Test case finished with exit code set to 42. + at exitSanitizer (ext:cli/40_test.js:113:15) + at async outerWrapped (ext:cli/40_test.js:134:14) + + FAILURES + +Deno.exitCode => ./main.js:1:6 + +FAILED | 1 passed | 1 failed ([WILDCARD]) + +error: Test failed diff --git a/tests/unit_node/process_test.ts b/tests/unit_node/process_test.ts index 8f56c92a0b..5e2fb69c24 100644 --- a/tests/unit_node/process_test.ts +++ b/tests/unit_node/process_test.ts @@ -787,10 +787,10 @@ Deno.test("process.exitCode", () => { assertEquals(process.exitCode, undefined); process.exitCode = 127; assertEquals(process.exitCode, 127); - // deno-lint-ignore no-explicit-any - (process.exitCode as any) = "asdf"; - // deno-lint-ignore no-explicit-any - assertEquals(process.exitCode as any, "asdf"); + assertThrows(() => { + // deno-lint-ignore no-explicit-any + (process.exitCode as any) = "asdf"; + }); // deno-lint-ignore no-explicit-any (process.exitCode as any) = "10"; process.exitCode = undefined; // reset @@ -827,7 +827,7 @@ Deno.test("process.exitCode in should change exit code", async () => { ); await exitCodeTest( "import process from 'node:process'; process.exitCode = NaN;", - 0, + 1, ); });