node:fs: accept any object with aborted as options.signal in readFile, writeFile and appendFile

mordant-baseline.toml

@@ -158,6 +158,7 @@
 "error_collapsed_to_bool:src/runtime/api/bun/Terminal.rs" = 1
 "error_collapsed_to_bool:src/runtime/api/bun/h2_frame_parser.rs" = 29
 "error_collapsed_to_bool:src/runtime/ffi/ffi_body.rs" = 1
+"field_valid_only_when:src/runtime/node/node_fs.rs" = 1
 "field_valid_only_when:src/runtime/node/zlib/NativeZlib.rs" = 1
 "field_valid_only_when:src/runtime/shell/builtin/rm.rs" = 1
 "field_valid_only_when:src/runtime/socket/udp_socket.rs" = 1

src/js/node/fs.promises.ts

@@ -515,22 +515,27 @@ function asyncWrap(fn: (...args: any[]) => Promise<unknown>, name: string) {
     // needs to exist for https://github.com/nodejs/node/blob/8641d941893/test/parallel/test-worker-message-port-transfer-fake-js-transferable.js to pass
     [Symbol("messaging_transfer_symbol")]() {}
 
-    async appendFile(data, options?: BufferEncoding | { encoding?: BufferEncoding | null; flush?: boolean } | null) {
+    async appendFile(
+      data,
+      options?: BufferEncoding | { encoding?: BufferEncoding | null; flush?: boolean; signal?: AbortSignal } | null,
+    ) {
       const fd = this[kFd];
       throwEBADFIfNecessary("writeFile", fd);
       let encoding: BufferEncoding = "utf8";
       let flush = false;
+      let signal: AbortSignal | undefined = undefined;
       if (options == null || typeof options === "function") {
       } else if (typeof options === "string") {
         encoding = options;
       } else {
         encoding = options?.encoding ?? encoding;
         flush = options?.flush ?? flush;
+        signal = options?.signal ?? undefined;
       }
 
       try {
         this[kRef]();
-        return await writeFile(fd, data, { encoding, flush, flag: this[kFlag] });
+        return await writeFile(fd, data, { encoding, flush, flag: this[kFlag], signal });
       } finally {
         this[kUnref]();
       }
@@ -1694,10 +1699,9 @@ async function writeFileAsyncIterator(fdOrPath, iterable, optionsOrEncoding, fla
     encoding = optionsOrEncoding?.encoding ?? (encoding || "utf8");
     flag = optionsOrEncoding?.flag ?? (flag || "w");
     mode = optionsOrEncoding?.mode ?? (mode || 0o666);
-    signal = optionsOrEncoding?.signal ?? null;
-    if (signal?.aborted) {
-      throw $makeAbortError(undefined, { cause: signal.reason });
-    }
+    // As the binding has it for a string or a Buffer: undefined, null and "" are no signal. Node takes only undefined for none.
+    const given = optionsOrEncoding?.signal;
+    signal = given === "" ? null : (given ?? null);
   } else if (typeof optionsOrEncoding === "string" || optionsOrEncoding == null) {
     encoding = optionsOrEncoding || "utf8";
     flag ??= "w";
@@ -1709,6 +1713,11 @@ async function writeFileAsyncIterator(fdOrPath, iterable, optionsOrEncoding, fla
     throw new TypeError(`Unknown encoding: ${encoding}`);
   }
 
+  if (signal !== null) validateAbortSignal(signal, "options.signal");
+  if (signal?.aborted) {
+    throw $makeAbortError(undefined, { cause: signal.reason });
+  }
+
   let mustClose = typeof fdOrPath === "string";
   if (mustClose) {
     // Rely on fs.open for further argument validaiton.

src/jsc/AbortSignal.rs

@@ -137,12 +137,17 @@ impl AbortSignal {
         }
         // `jsReason` returns the same (cached) object the JS `.reason` getter
         // does, so `err.cause === signal.reason` holds, matching Node.
-        Some(Bun__wrapAbortError(
+        Some(Self::node_abort_error(
             global,
             WebCore__AbortSignal__jsReason(self, global),
         ))
     }
 
+    /// Node's `new AbortError(undefined, { cause })`, for the reason of any signal.
+    pub fn node_abort_error(global: &JSGlobalObject, cause: JSValue) -> JSValue {
+        Bun__wrapAbortError(global, cause)
+    }
+
     /// Take a ref on the signal.
     pub fn ref_(&self) -> AbortSignalRef {
         // SAFETY: `WebCore__AbortSignal__ref` bumps the intrusive refcount and

src/jsc/bindings/ErrorCode.cpp

@@ -2009,10 +2009,6 @@ extern "C" JSC::EncodedJSValue Bun__wrapAbortError(JSC::JSGlobalObject* lexicalG
     auto& vm = JSC::getVM(globalObject);
     auto cause = JSC::JSValue::decode(causeParam);
 
-    if (cause.isUndefined()) {
-        return JSC::JSValue::encode(Bun::createError(vm, globalObject, Bun::ErrorCode::ABORT_ERR, Bun::commonStrings(vm).OperationWasAbortedString()));
-    }
-
     auto message = Bun::commonStrings(vm).OperationWasAbortedString();
     JSC::JSObject* options = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 24);
     Bun::putDirectNamed(vm, options, "cause"_s, cause);

src/jsc/bindings/NodeValidator.cpp

@@ -407,6 +407,13 @@ extern "C" SYSV_ABI JSC::EncodedJSValue Bun__NodeValidator__validateObject(JSC::
     return NodeValidators::validateObject(globalObject, Bindgen::ArgumentsInRegisters<3> { encodedValue, encodedName, encodedOptions });
 }
 
+// For a native parser of options whose own test for the class has failed: validate_abort_signal in validators.rs.
+extern "C" [[ZIG_EXPORT(false_is_throw)]] bool Bun__validateNonNativeAbortSignal(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue encodedSignal)
+{
+    auto scope = DECLARE_THROW_SCOPE(JSC::getVM(globalObject));
+    return NodeValidators::validateNonNativeAbortSignal(scope, globalObject, JSValue::decode(encodedSignal), [] { return WTF::String("options.signal"_s); });
+}
+
 //
 //
 

src/jsc/bindings/NodeValidatorBodies.h

@@ -212,6 +212,20 @@ template<typename Arguments> ALWAYS_INLINE JSC::EncodedJSValue validatePort(JSC:
     return JSValue::encode(jsNumber(static_cast<int32_t>(port_num)));
 }
 
+// Node's validateAbortSignal for a value that is no AbortSignal of Bun's. False: it threw Node's error, or a trap of a proxy threw.
+template<typename NameOf> ALWAYS_INLINE bool validateNonNativeAbortSignal(JSC::ThrowScope& scope, JSC::JSGlobalObject* globalObject, JSValue signal, const NameOf& nameOf)
+{
+    auto* object = signal.getObject();
+    bool isSignal = object && !object->isCallable() && object->hasProperty(globalObject, Identifier::fromString(JSC::getVM(globalObject), "aborted"_s));
+    RETURN_IF_EXCEPTION(scope, false);
+    if (isSignal) return true;
+
+    WTF::String name = nameOf();
+    RETURN_IF_EXCEPTION(scope, false);
+    Bun::ERR::INVALID_ARG_INSTANCE(scope, globalObject, name, "AbortSignal"_s, signal);
+    return false;
+}
+
 template<typename Arguments> ALWAYS_INLINE JSC::EncodedJSValue validateAbortSignal(JSC::JSGlobalObject* globalObject, const Arguments& arguments)
 {
     auto& vm = JSC::getVM(globalObject);
@@ -220,16 +234,8 @@ template<typename Arguments> ALWAYS_INLINE JSC::EncodedJSValue validateAbortSign
     JSValue signal = arguments.at(0);
     JSValue name = arguments.at(1);
 
-    if (!signal.isUndefined()) {
-        auto* object = signal.getObject();
-        if (!object) return Bun::ERR::INVALID_ARG_TYPE(scope, globalObject, name, "AbortSignal"_s, signal);
-        if (object->inherits<WebCore::JSAbortSignal>()) {
-            return JSValue::encode(jsUndefined());
-        }
-
-        auto propin = object->hasProperty(globalObject, Identifier::fromString(vm, "aborted"_s));
-        RETURN_IF_EXCEPTION(scope, {});
-        if (!propin) return Bun::ERR::INVALID_ARG_TYPE(scope, globalObject, name, "AbortSignal"_s, signal);
+    if (!signal.isUndefined() && !signal.inherits<WebCore::JSAbortSignal>()) {
+        if (!validateNonNativeAbortSignal(scope, globalObject, signal, [&] { return name.toWTFString(globalObject); })) return {};
     }
 
     return JSValue::encode(jsUndefined());