fix(std/node): Stop callbacks being called twice when callback throws error (#8867)

std/node/README.md

@@ -82,3 +82,56 @@ const cjsModule = require("./my_mod");
 // Visits node_modules.
 const leftPad = require("left-pad");
 ```
+
+## Contributing
+
+When converting from promise-based to callback-based APIs, the most obvious way
+is like this:
+
+```ts
+promise.then((value) => callback(null, value)).catch(callback);
+```
+
+This has a subtle bug - if the callback throws an error, the catch statement
+will also catch _that_ error, and the callback will be called twice. The correct
+way to do it is like this:
+
+```ts
+promise.then((value) => callback(null, value), callback);
+```
+
+The second parameter of `then` can also be used to catch errors, but only errors
+from the existing promise, not the new one created by the callback.
+
+If the Deno equivalent is actually synchronous, there's a similar problem with
+try/catch statements:
+
+```ts
+try {
+  const value = process();
+  callback(null, value);
+} catch (err) {
+  callback(err);
+}
+```
+
+Since the callback is called within the `try` block, any errors from it will be
+caught and call the callback again.
+
+The correct way to do it is like this:
+
+```ts
+let err, value;
+try {
+  value = process();
+} catch (e) {
+  err = e;
+}
+if (err) {
+  callback(err); // Make sure arguments.length === 1
+} else {
+  callback(null, value);
+}
+```
+
+It's not as clean, but prevents the callback being called twice.

std/node/_crypto/pbkdf2.ts

@@ -162,19 +162,25 @@ export function pbkdf2(
   iterations: number,
   keylen: number,
   digest: Algorithms = "sha1",
-  callback: ((err?: Error, derivedKey?: Buffer) => void),
+  callback: ((err: Error | null, derivedKey?: Buffer) => void),
 ): void {
+  setTimeout(() => {
+    let err = null, res;
     try {
-    const res = pbkdf2Sync(
+      res = pbkdf2Sync(
         password,
         salt,
         iterations,
         keylen,
         digest,
       );
-
-    callback(undefined, res);
     } catch (e) {
-    callback(e);
+      err = e;
     }
+    if (err) {
+      callback(err);
+    } else {
+      callback(null, res);
+    }
+  }, 0);
 }

std/node/_crypto/pbkdf2_test.ts

@@ -3,7 +3,12 @@ import {
   pbkdf2,
   pbkdf2Sync,
 } from "./pbkdf2.ts";
-import { assert, assertEquals } from "../../testing/asserts.ts";
+import {
+  assert,
+  assertEquals,
+  assertStringIncludes,
+} from "../../testing/asserts.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 
 type Pbkdf2Fixture = {
   key: string | Float64Array | Int32Array | Uint8Array;
@@ -412,3 +417,11 @@ Deno.test("pbkdf2Sync hashes data correctly", () => {
     }
   });
 });
+
+Deno.test("[std/node/crypto] pbkdf2 callback isn't called twice if error is thrown", async () => {
+  const importUrl = new URL("./pbkdf2.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import { pbkdf2 } from ${JSON.stringify(importUrl)}`,
+    invocation: 'pbkdf2("password", "salt", 1, 32, "sha1", ',
+  });
+});

std/node/_crypto/randomBytes.ts

@@ -39,8 +39,9 @@ export default function randomBytes(
   cb?: (err: Error | null, buf?: Buffer) => void,
 ): Buffer | void {
   if (typeof cb === "function") {
+    let err: Error | null = null, bytes: Buffer;
     try {
-      cb(null, generateRandomBytes(size));
+      bytes = generateRandomBytes(size);
     } catch (e) {
       //NodeJS nonsense
       //If the size is out of range it will throw sync, otherwise throw async
@@ -50,9 +51,16 @@ export default function randomBytes(
       ) {
         throw e;
       } else {
-        cb(e);
+        err = e;
       }
     }
+    setTimeout(() => {
+      if (err) {
+        cb(err);
+      } else {
+        cb(null, bytes);
+      }
+    }, 0);
   } else {
     return generateRandomBytes(size);
   }

std/node/_crypto/randomBytes_test.ts

@@ -1,4 +1,11 @@
-import { assert, assertEquals, assertThrows } from "../../testing/asserts.ts";
+import {
+  assert,
+  assertEquals,
+  assertStringIncludes,
+  assertThrows,
+  assertThrowsAsync,
+} from "../../testing/asserts.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import randomBytes, { MAX_RANDOM_VALUES, MAX_SIZE } from "./randomBytes.ts";
 
 Deno.test("randomBytes sync works correctly", function () {
@@ -59,10 +66,24 @@ Deno.test("randomBytes async works correctly", function () {
       assert(!err);
     })
   );
-  assertThrows(() =>
+  assertThrowsAsync(() =>
-    randomBytes(-1, function (err) {
+    new Promise((resolve, reject) => {
+      randomBytes(-1, function (err, res) {
         //Shouldn't throw async
-      assert(!err);
+        if (err) {
+          reject(err);
+        } else {
+          resolve(res);
+        }
+      });
     })
   );
 });
+
+Deno.test("[std/node/crypto] randomBytes callback isn't called twice if error is thrown", async () => {
+  const importUrl = new URL("./randomBytes.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import randomBytes from ${JSON.stringify(importUrl)}`,
+    invocation: "randomBytes(0, ",
+  });
+});

std/node/_fs/_fs_appendFile.ts

@@ -35,7 +35,7 @@ export function appendFile(
   new Promise((resolve, reject) => {
     if (typeof pathOrRid === "number") {
       rid = pathOrRid;
-      Deno.write(rid, buffer).then(resolve).catch(reject);
+      Deno.write(rid, buffer).then(resolve, reject);
     } else {
       const mode: number | undefined = isFileOptions(options)
         ? options.mode
@@ -53,15 +53,13 @@ export function appendFile(
           rid = openedFileRid;
           return Deno.write(openedFileRid, buffer);
         })
-        .then(resolve)
+        .then(resolve, reject);
-        .catch(reject);
     }
   })
     .then(() => {
       closeRidIfNecessary(typeof pathOrRid === "string", rid);
-      callbackFn();
+      callbackFn(null);
-    })
+    }, (err) => {
-    .catch((err) => {
       closeRidIfNecessary(typeof pathOrRid === "string", rid);
       callbackFn(err);
     });

std/node/_fs/_fs_appendFile_test.ts

@@ -2,6 +2,7 @@
 import { assertEquals, assertThrows, fail } from "../../testing/asserts.ts";
 import { appendFile, appendFileSync } from "./_fs_appendFile.ts";
 import { fromFileUrl } from "../path.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 
 const decoder = new TextDecoder("utf-8");
 
@@ -78,8 +79,7 @@ Deno.test({
       .then(async () => {
         const data = await Deno.readFile(tempFile);
         assertEquals(decoder.decode(data), "hello world");
-      })
+      }, () => {
-      .catch(() => {
         fail("No error expected");
       })
       .finally(async () => {
@@ -103,8 +103,7 @@ Deno.test({
         assertEquals(Deno.resources(), openResourcesBeforeAppend);
         const data = await Deno.readFile("_fs_appendFile_test_file.txt");
         assertEquals(decoder.decode(data), "hello world");
-      })
+      }, (err) => {
-      .catch((err) => {
         fail("No error was expected: " + err);
       })
       .finally(async () => {
@@ -128,8 +127,7 @@ Deno.test({
         assertEquals(Deno.resources(), openResourcesBeforeAppend);
         const data = await Deno.readFile(fromFileUrl(fileURL));
         assertEquals(decoder.decode(data), "hello world");
-      })
+      }, (err) => {
-      .catch((err) => {
         fail("No error was expected: " + err);
       })
       .finally(async () => {
@@ -152,8 +150,7 @@ Deno.test({
     })
       .then(() => {
         fail("Expected error to be thrown");
-      })
+      }, () => {
-      .catch(() => {
         assertEquals(Deno.resources(), openResourcesBeforeAppend);
       })
       .finally(async () => {
@@ -235,8 +232,7 @@ Deno.test({
         assertEquals(Deno.resources(), openResourcesBeforeAppend);
         const data = await Deno.readFile("_fs_appendFile_test_file.txt");
         assertEquals(data, testData);
-      })
+      }, (err) => {
-      .catch((err) => {
         fail("No error was expected: " + err);
       })
       .finally(async () => {
@@ -244,3 +240,15 @@ Deno.test({
       });
   },
 });
+
+Deno.test("[std/node/fs] appendFile callback isn't called twice if error is thrown", async () => {
+  const tempFile = await Deno.makeTempFile();
+  const importUrl = new URL("./_fs_appendFile.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import { appendFile } from ${JSON.stringify(importUrl)}`,
+    invocation: `appendFile(${JSON.stringify(tempFile)}, "hello world", `,
+    async cleanup() {
+      await Deno.remove(tempFile);
+    },
+  });
+});

std/node/_fs/_fs_chmod.ts

@@ -15,9 +15,7 @@ export function chmod(
 ): void {
   path = path instanceof URL ? fromFileUrl(path) : path;
 
-  Deno.chmod(path, getResolvedMode(mode))
+  Deno.chmod(path, getResolvedMode(mode)).then(() => callback(null), callback);
-    .then(() => callback())
-    .catch(callback);
 }
 
 /**

std/node/_fs/_fs_chmod_test.ts

@@ -1,5 +1,6 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
 import { assert, fail } from "../../testing/asserts.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import { chmod, chmodSync } from "./_fs_chmod.ts";
 
 Deno.test({
@@ -18,8 +19,7 @@ Deno.test({
         const newFileMode: number | null = Deno.lstatSync(tempFile).mode;
         assert(newFileMode && originalFileMode);
         assert(newFileMode === 33279 && newFileMode > originalFileMode);
-      })
+      }, () => {
-      .catch(() => {
         fail();
       })
       .finally(() => {
@@ -42,3 +42,19 @@ Deno.test({
     Deno.removeSync(tempFile);
   },
 });
+
+Deno.test({
+  name: "[std/node/fs] chmod callback isn't called twice if error is thrown",
+  ignore: Deno.build.os === "windows",
+  async fn() {
+    const tempFile = await Deno.makeTempFile();
+    const importUrl = new URL("./_fs_chmod.ts", import.meta.url);
+    await assertCallbackErrorUncaught({
+      prelude: `import { chmod } from ${JSON.stringify(importUrl)}`,
+      invocation: `chmod(${JSON.stringify(tempFile)}, 0o777, `,
+      async cleanup() {
+        await Deno.remove(tempFile);
+      },
+    });
+  },
+});

std/node/_fs/_fs_chown.ts

@@ -14,9 +14,7 @@ export function chown(
 ): void {
   path = path instanceof URL ? fromFileUrl(path) : path;
 
-  Deno.chown(path, uid, gid)
+  Deno.chown(path, uid, gid).then(() => callback(null), callback);
-    .then(() => callback())
-    .catch(callback);
 }
 
 /**

std/node/_fs/_fs_chown_test.ts

@@ -1,5 +1,6 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
 import { assertEquals, fail } from "../../testing/asserts.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import { chown, chownSync } from "./_fs_chown.ts";
 
 // chown is difficult to test.  Best we can do is set the existing user id/group
@@ -24,8 +25,7 @@ Deno.test({
         const newGroupId: number | null = Deno.lstatSync(tempFile).gid;
         assertEquals(newUserId, originalUserId);
         assertEquals(newGroupId, originalGroupId);
-      })
+      }, () => {
-      .catch(() => {
         fail();
       })
       .finally(() => {
@@ -50,3 +50,20 @@ Deno.test({
     Deno.removeSync(tempFile);
   },
 });
+
+Deno.test({
+  name: "[std/node/fs] chown callback isn't called twice if error is thrown",
+  ignore: Deno.build.os === "windows",
+  async fn() {
+    const tempFile = await Deno.makeTempFile();
+    const { uid, gid } = await Deno.lstat(tempFile);
+    const importUrl = new URL("./_fs_chown.ts", import.meta.url);
+    await assertCallbackErrorUncaught({
+      prelude: `import { chown } from ${JSON.stringify(importUrl)}`,
+      invocation: `chown(${JSON.stringify(tempFile)}, ${uid}, ${gid}, `,
+      async cleanup() {
+        await Deno.remove(tempFile);
+      },
+    });
+  },
+});

std/node/_fs/_fs_close.ts

@@ -2,14 +2,15 @@
 import type { CallbackWithError } from "./_fs_common.ts";
 
 export function close(fd: number, callback: CallbackWithError): void {
-  queueMicrotask(() => {
+  setTimeout(() => {
+    let error = null;
     try {
       Deno.close(fd);
-      callback(null);
     } catch (err) {
-      callback(err);
+      error = err;
     }
-  });
+    callback(error);
+  }, 0);
 }
 
 export function closeSync(fd: number): void {

std/node/_fs/_fs_close_test.ts

@@ -1,5 +1,6 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
 import { assert, assertThrows, fail } from "../../testing/asserts.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import { close, closeSync } from "./_fs_close.ts";
 
 Deno.test({
@@ -17,8 +18,7 @@ Deno.test({
     })
       .then(() => {
         assert(!Deno.resources()[file.rid]);
-      })
+      }, () => {
-      .catch(() => {
         fail("No error expected");
       })
       .finally(async () => {
@@ -78,3 +78,19 @@ Deno.test({
     assertThrows(() => closeSync(-1));
   },
 });
+
+Deno.test("[std/node/fs] close callback isn't called twice if error is thrown", async () => {
+  const tempFile = await Deno.makeTempFile();
+  const importUrl = new URL("./_fs_close.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `
+    import { close } from ${JSON.stringify(importUrl)};
+
+    const file = await Deno.open(${JSON.stringify(tempFile)});
+    `,
+    invocation: "close(file.rid, ",
+    async cleanup() {
+      await Deno.remove(tempFile);
+    },
+  });
+});

std/node/_fs/_fs_common.ts

@@ -6,7 +6,7 @@ import {
   TextEncodings,
 } from "../_utils.ts";
 
-export type CallbackWithError = (err?: Error | null) => void;
+export type CallbackWithError = (err: Error | null) => void;
 
 export interface FileOptions {
   encoding?: Encodings;

std/node/_fs/_fs_copy.ts

@@ -9,9 +9,7 @@ export function copyFile(
 ): void {
   source = source instanceof URL ? fromFileUrl(source) : source;
 
-  Deno.copyFile(source, destination)
+  Deno.copyFile(source, destination).then(() => callback(null), callback);
-    .then(() => callback())
-    .catch(callback);
 }
 
 export function copyFileSync(source: string | URL, destination: string): void {

std/node/_fs/_fs_copy_test.ts

@@ -1,5 +1,7 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
+import * as path from "../../path/mod.ts";
 import { assert } from "../../testing/asserts.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import { copyFile, copyFileSync } from "./_fs_copy.ts";
 import { existsSync } from "./_fs_exists.ts";
 
@@ -29,3 +31,23 @@ Deno.test({
     Deno.removeSync(destFile);
   },
 });
+
+Deno.test("[std/node/fs] copyFile callback isn't called twice if error is thrown", async () => {
+  // The correct behaviour is not to catch any errors thrown,
+  // but that means there'll be an uncaught error and the test will fail.
+  // So the only way to test this is to spawn a subprocess, and succeed if it has a non-zero exit code.
+  // (assertThrowsAsync won't work because there's no way to catch the error.)
+  const tempDir = await Deno.makeTempDir();
+  const tempFile1 = path.join(tempDir, "file1.txt");
+  const tempFile2 = path.join(tempDir, "file2.txt");
+  await Deno.writeTextFile(tempFile1, "hello world");
+  const importUrl = new URL("./_fs_copy.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import { copyFile } from ${JSON.stringify(importUrl)}`,
+    invocation: `copyFile(${JSON.stringify(tempFile1)}, 
+                          ${JSON.stringify(tempFile2)}, `,
+    async cleanup() {
+      await Deno.remove(tempDir, { recursive: true });
+    },
+  });
+});

std/node/_fs/_fs_dir.ts

@@ -32,10 +32,9 @@ export default class Dir {
           if (callback) {
             callback(null, value ? value : null);
           }
-        })
+        }, (err) => {
-        .catch((err) => {
           if (callback) {
-            callback(err, null);
+            callback(err);
           }
           reject(err);
         });
@@ -59,18 +58,11 @@ export default class Dir {
    */
   // deno-lint-ignore no-explicit-any
   close(callback?: (...args: any[]) => void): Promise<void> {
-    return new Promise((resolve, reject) => {
+    return new Promise((resolve) => {
-      try {
       if (callback) {
         callback(null);
       }
       resolve();
-      } catch (err) {
-        if (callback) {
-          callback(err);
-        }
-        reject(err);
-      }
     });
   }
 

std/node/_fs/_fs_dir_test.ts

@@ -1,5 +1,6 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
 import { assert, assertEquals, fail } from "../../testing/asserts.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import Dir from "./_fs_dir.ts";
 import type Dirent from "./_fs_dirent.ts";
 
@@ -165,3 +166,35 @@ Deno.test({
     }
   },
 });
+
+Deno.test("[std/node/fs] Dir.close callback isn't called twice if error is thrown", async () => {
+  const tempDir = await Deno.makeTempDir();
+  const importUrl = new URL("./_fs_dir.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `
+    import Dir from ${JSON.stringify(importUrl)};
+
+    const dir = new Dir(${JSON.stringify(tempDir)});
+    `,
+    invocation: "dir.close(",
+    async cleanup() {
+      await Deno.remove(tempDir);
+    },
+  });
+});
+
+Deno.test("[std/node/fs] Dir.read callback isn't called twice if error is thrown", async () => {
+  const tempDir = await Deno.makeTempDir();
+  const importUrl = new URL("./_fs_dir.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `
+    import Dir from ${JSON.stringify(importUrl)};
+
+    const dir = new Dir(${JSON.stringify(tempDir)});
+    `,
+    invocation: "dir.read(",
+    async cleanup() {
+      await Deno.remove(tempDir);
+    },
+  });
+});

std/node/_fs/_fs_exists.ts

@@ -10,11 +10,7 @@ type ExitsCallback = (exists: boolean) => void;
  */
 export function exists(path: string | URL, callback: ExitsCallback): void {
   path = path instanceof URL ? fromFileUrl(path) : path;
-  Deno.lstat(path)
+  Deno.lstat(path).then(() => callback(true), () => callback(false));
-    .then(() => {
-      callback(true);
-    })
-    .catch(() => callback(false));
 }
 
 /**

std/node/_fs/_fs_exists_test.ts

@@ -1,5 +1,9 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
-import { assertEquals } from "../../testing/asserts.ts";
+import {
+  assert,
+  assertEquals,
+  assertStringIncludes,
+} from "../../testing/asserts.ts";
 import { exists, existsSync } from "./_fs_exists.ts";
 
 Deno.test("existsFile", async function () {
@@ -23,3 +27,32 @@ Deno.test("existsSyncFile", function () {
   Deno.removeSync(tmpFilePath);
   assertEquals(existsSync("./notAvailable.txt"), false);
 });
+
+Deno.test("[std/node/fs] exists callback isn't called twice if error is thrown", async () => {
+  // This doesn't use `assertCallbackErrorUncaught()` because `exists()` doesn't return a standard node callback, which is what it expects.
+  const tempFile = await Deno.makeTempFile();
+  const importUrl = new URL("./_fs_exists.ts", import.meta.url);
+  const p = Deno.run({
+    cmd: [
+      Deno.execPath(),
+      "eval",
+      "--no-check",
+      `
+      import { exists } from ${JSON.stringify(importUrl)};
+
+      exists(${JSON.stringify(tempFile)}, (exists) => {
+        // If the bug is present and the callback is called again with false (meaning an error occured),
+        // don't throw another error, so if the subprocess fails we know it had the correct behaviour.
+        if (exists) throw new Error("success");
+      });`,
+    ],
+    stderr: "piped",
+  });
+  const status = await p.status();
+  const stderr = new TextDecoder().decode(await Deno.readAll(p.stderr));
+  p.close();
+  p.stderr.close();
+  await Deno.remove(tempFile);
+  assert(!status.success);
+  assertStringIncludes(stderr, "Error: success");
+});

std/node/_fs/_fs_link.ts

@@ -16,9 +16,7 @@ export function link(
     : existingPath;
   newPath = newPath instanceof URL ? fromFileUrl(newPath) : newPath;
 
-  Deno.link(existingPath, newPath)
+  Deno.link(existingPath, newPath).then(() => callback(null), callback);
-    .then(() => callback())
-    .catch(callback);
 }
 
 /**

std/node/_fs/_fs_link_test.ts

@@ -1,7 +1,8 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
-import { assertEquals, fail } from "../../testing/asserts.ts";
+import * as path from "../../path/mod.ts";
+import { assert, assertEquals, fail } from "../../testing/asserts.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import { link, linkSync } from "./_fs_link.ts";
-import { assert } from "../../testing/asserts.ts";
 
 Deno.test({
   name: "ASYNC: hard linking files works as expected",
@@ -16,8 +17,7 @@ Deno.test({
     })
       .then(() => {
         assertEquals(Deno.statSync(tempFile), Deno.statSync(linkedFile));
-      })
+      }, () => {
-      .catch(() => {
         fail("Expected to succeed");
       })
       .finally(() => {
@@ -39,8 +39,7 @@ Deno.test({
     })
       .then(() => {
         fail("Expected to succeed");
-      })
+      }, (err) => {
-      .catch((err) => {
         assert(err);
         failed = true;
       });
@@ -60,3 +59,19 @@ Deno.test({
     Deno.removeSync(linkedFile);
   },
 });
+
+Deno.test("[std/node/fs] link callback isn't called twice if error is thrown", async () => {
+  const tempDir = await Deno.makeTempDir();
+  const tempFile = path.join(tempDir, "file.txt");
+  const linkFile = path.join(tempDir, "link.txt");
+  await Deno.writeTextFile(tempFile, "hello world");
+  const importUrl = new URL("./_fs_link.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import { link } from ${JSON.stringify(importUrl)}`,
+    invocation: `link(${JSON.stringify(tempFile)}, 
+                      ${JSON.stringify(linkFile)}, `,
+    async cleanup() {
+      await Deno.remove(tempDir, { recursive: true });
+    },
+  });
+});

std/node/_fs/_fs_lstat.ts

@@ -27,8 +27,7 @@ export function lstat(
     (typeof optionsOrCallback === "function"
       ? optionsOrCallback
       : maybeCallback) as (
-        err: Error | undefined,
+        ...args: [Error] | [null, BigIntStats | Stats]
-        stat: BigIntStats | Stats,
       ) => void;
   const options = typeof optionsOrCallback === "object"
     ? optionsOrCallback
@@ -36,9 +35,10 @@ export function lstat(
 
   if (!callback) throw new Error("No callback function supplied");
 
-  Deno.lstat(path)
+  Deno.lstat(path).then(
-    .then((stat) => callback(undefined, CFISBIS(stat, options.bigint)))
+    (stat) => callback(null, CFISBIS(stat, options.bigint)),
-    .catch((err) => callback(err, err));
+    (err) => callback(err),
+  );
 }
 
 export function lstatSync(path: string | URL): Stats;

std/node/_fs/_fs_lstat_test.ts

@@ -1,5 +1,6 @@
 import { lstat, lstatSync } from "./_fs_lstat.ts";
 import { fail } from "../../testing/asserts.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import { assertStats, assertStatsBigInt } from "./_fs_stat_test.ts";
 import type { BigIntStats, Stats } from "./_fs_stat.ts";
 
@@ -15,8 +16,7 @@ Deno.test({
     })
       .then((stat) => {
         assertStats(stat, Deno.lstatSync(file));
-      })
+      }, () => fail())
-      .catch(() => fail())
       .finally(() => {
         Deno.removeSync(file);
       });
@@ -41,8 +41,10 @@ Deno.test({
         resolve(stat);
       });
     })
-      .then((stat) => assertStatsBigInt(stat, Deno.lstatSync(file)))
+      .then(
-      .catch(() => fail())
+        (stat) => assertStatsBigInt(stat, Deno.lstatSync(file)),
+        () => fail(),
+      )
       .finally(() => Deno.removeSync(file));
   },
 });
@@ -54,3 +56,15 @@ Deno.test({
     assertStatsBigInt(lstatSync(file, { bigint: true }), Deno.lstatSync(file));
   },
 });
+
+Deno.test("[std/node/fs] lstat callback isn't called twice if error is thrown", async () => {
+  const tempFile = await Deno.makeTempFile();
+  const importUrl = new URL("./_fs_lstat.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import { lstat } from ${JSON.stringify(importUrl)}`,
+    invocation: `lstat(${JSON.stringify(tempFile)}, `,
+    async cleanup() {
+      await Deno.remove(tempFile);
+    },
+  });
+});

std/node/_fs/_fs_mkdir.ts

@@ -39,10 +39,9 @@ export function mkdir(
   Deno.mkdir(path, { recursive, mode })
     .then(() => {
       if (typeof callback === "function") {
-        callback();
+        callback(null);
       }
-    })
+    }, (err) => {
-    .catch((err) => {
       if (typeof callback === "function") {
         callback(err);
       }

std/node/_fs/_fs_mkdir_test.ts

@@ -1,5 +1,7 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
+import * as path from "../../path/mod.ts";
 import { assert } from "../../testing/asserts.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import { mkdir, mkdirSync } from "./_fs_mkdir.ts";
 import { existsSync } from "./_fs_exists.ts";
 
@@ -27,3 +29,16 @@ Deno.test({
     Deno.removeSync(tmpDir);
   },
 });
+
+Deno.test("[std/node/fs] mkdir callback isn't called twice if error is thrown", async () => {
+  const tempDir = await Deno.makeTempDir();
+  const subdir = path.join(tempDir, "subdir");
+  const importUrl = new URL("./_fs_mkdir.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import { mkdir } from ${JSON.stringify(importUrl)}`,
+    invocation: `mkdir(${JSON.stringify(subdir)}, `,
+    async cleanup() {
+      await Deno.remove(tempDir, { recursive: true });
+    },
+  });
+});

std/node/_fs/_fs_mkdtemp.ts

@@ -7,7 +7,7 @@ import {
 } from "../_errors.ts";
 
 export type mkdtempCallback = (
-  err: Error | undefined,
+  err: Error | null,
   directory?: string,
 ) => void;
 
@@ -35,7 +35,7 @@ export function mkdtemp(
     { recursive: false, mode: 0o700 },
     (err: Error | null | undefined) => {
       if (err) callback(err);
-      else callback(undefined, decode(path, encoding));
+      else callback(null, decode(path, encoding));
     },
   );
 }

std/node/_fs/_fs_open.ts

@@ -17,7 +17,7 @@ type openFlags =
   | "w+"
   | "wx+";
 
-type openCallback = (err: Error | undefined, fd: number) => void;
+type openCallback = (err: Error | null, fd: number) => void;
 
 function convertFlagAndModeToOptions(
   flag?: openFlags,
@@ -61,20 +61,26 @@ export function open(
 
   if (["ax", "ax+", "wx", "wx+"].includes(flags || "") && existsSync(path)) {
     const err = new Error(`EEXIST: file already exists, open '${path}'`);
-    callback(err, 0);
+    (callback as (err: Error) => void)(err);
   } else {
     if (flags === "as" || flags === "as+") {
+      let err: Error | null = null, res: number;
       try {
-        const res = openSync(path, flags, mode);
+        res = openSync(path, flags, mode);
-        callback(undefined, res);
       } catch (error) {
-        callback(error, error);
+        err = error;
+      }
+      if (err) {
+        (callback as (err: Error) => void)(err);
+      } else {
+        callback(null, res!);
       }
       return;
     }
-    Deno.open(path, convertFlagAndModeToOptions(flags, mode))
+    Deno.open(path, convertFlagAndModeToOptions(flags, mode)).then(
-      .then((file) => callback(undefined, file.rid))
+      (file) => callback(null, file.rid),
-      .catch((err) => callback(err, err));
+      (err) => (callback as (err: Error) => void)(err),
+    );
   }
 }
 

std/node/_fs/_fs_open_test.ts

@@ -4,6 +4,7 @@ import {
   assertThrows,
   fail,
 } from "../../testing/asserts.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import { open, openSync } from "./_fs_open.ts";
 import { join, parse } from "../../path/mod.ts";
 import { existsSync } from "../../fs/mod.ts";
@@ -25,8 +26,7 @@ Deno.test({
       .then((fd) => {
         fd1 = fd;
         assert(Deno.resources()[fd], `${fd}`);
-      })
+      }, () => fail())
-      .catch(() => fail())
       .finally(() => closeSync(fd1));
   },
 });
@@ -207,3 +207,15 @@ Deno.test({
     Deno.removeSync(file);
   },
 });
+
+Deno.test("[std/node/fs] open callback isn't called twice if error is thrown", async () => {
+  const tempFile = await Deno.makeTempFile();
+  const importUrl = new URL("./_fs_open.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import { open } from ${JSON.stringify(importUrl)}`,
+    invocation: `open(${JSON.stringify(tempFile)}, `,
+    async cleanup() {
+      await Deno.remove(tempFile);
+    },
+  });
+});

std/node/_fs/_fs_readFile.ts

@@ -69,7 +69,7 @@ export function readFile(
       }
       const buffer = maybeDecode(data, encoding);
       (cb as BinaryCallback)(null, buffer);
-    }).catch((err) => cb && cb(err));
+    }, (err) => cb && cb(err));
   }
 }
 

std/node/_fs/_fs_readFile_test.ts

@@ -1,4 +1,5 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import { readFile, readFileSync } from "./_fs_readFile.ts";
 import * as path from "../../path/mod.ts";
 import { assert, assertEquals } from "../../testing/asserts.ts";
@@ -103,3 +104,15 @@ Deno.test("readFileEncodeAsString", function () {
   assertEquals(typeof data, "string");
   assertEquals(data as string, "hello world");
 });
+
+Deno.test("[std/node/fs] readFile callback isn't called twice if error is thrown", async () => {
+  const tempFile = await Deno.makeTempFile();
+  const importUrl = new URL("./_fs_readFile.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import { readFile } from ${JSON.stringify(importUrl)}`,
+    invocation: `readFile(${JSON.stringify(tempFile)}, `,
+    async cleanup() {
+      await Deno.remove(tempFile);
+    },
+  });
+});

std/node/_fs/_fs_readdir.ts

@@ -11,13 +11,12 @@ type readDirOptions = {
   withFileTypes?: boolean;
 };
 
-type readDirCallback = (err: Error | undefined, files: string[]) => void;
+type readDirCallback = (err: Error | null, files: string[]) => void;
 
-type readDirCallbackDirent = (err: Error | undefined, files: Dirent[]) => void;
+type readDirCallbackDirent = (err: Error | null, files: Dirent[]) => void;
 
 type readDirBoth = (
-  err: Error | undefined,
+  ...args: [Error] | [null, string[] | Dirent[] | Array<string | Dirent>]
-  files: string[] | Dirent[] | Array<string | Dirent>,
 ) => void;
 
 export function readdir(
@@ -62,7 +61,7 @@ export function readdir(
     asyncIterableToCallback(Deno.readDir(path), (val, done) => {
       if (typeof path !== "string") return;
       if (done) {
-        callback(undefined, result);
+        callback(null, result);
         return;
       }
       if (options?.withFileTypes) {
@@ -70,7 +69,7 @@ export function readdir(
       } else result.push(decode(val.name));
     });
   } catch (error) {
-    callback(error, result);
+    callback(error);
   }
 }
 

std/node/_fs/_fs_readdir_test.ts

@@ -1,4 +1,5 @@
 import { assertEquals, assertNotEquals, fail } from "../../testing/asserts.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import { readdir, readdirSync } from "./_fs_readdir.ts";
 import { join } from "../../path/mod.ts";
 
@@ -12,8 +13,7 @@ Deno.test({
         resolve(files);
       });
     })
-      .then((files) => assertEquals(files, []))
+      .then((files) => assertEquals(files, []), () => fail())
-      .catch(() => fail())
       .finally(() => Deno.removeSync(dir));
   },
 });
@@ -40,10 +40,14 @@ Deno.test({
         resolve(files);
       });
     })
-      .then((files) =>
+      .then(
-        assertEqualsArrayAnyOrder(files, ["file1.txt", "some_dir", "file2.txt"])
+        (files) =>
+          assertEqualsArrayAnyOrder(
+            files,
+            ["file1.txt", "some_dir", "file2.txt"],
+          ),
+        () => fail(),
       )
-      .catch(() => fail())
       .finally(() => Deno.removeSync(dir, { recursive: true }));
   },
 });
@@ -69,3 +73,19 @@ Deno.test({
     );
   },
 });
+
+Deno.test("[std/node/fs] readdir callback isn't called twice if error is thrown", async () => {
+  // The correct behaviour is not to catch any errors thrown,
+  // but that means there'll be an uncaught error and the test will fail.
+  // So the only way to test this is to spawn a subprocess, and succeed if it has a non-zero exit code.
+  // (assertThrowsAsync won't work because there's no way to catch the error.)
+  const tempDir = await Deno.makeTempDir();
+  const importUrl = new URL("./_fs_readdir.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import { readdir } from ${JSON.stringify(importUrl)}`,
+    invocation: `readdir(${JSON.stringify(tempDir)}, `,
+    async cleanup() {
+      await Deno.remove(tempDir);
+    },
+  });
+});

std/node/_fs/_fs_readlink_test.ts

@@ -1,4 +1,5 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import { readlink, readlinkSync } from "./_fs_readlink.ts";
 import { assert, assertEquals } from "../../testing/asserts.ts";
 import * as path from "../path.ts";
@@ -64,3 +65,11 @@ Deno.test({
     assertEquals(new TextDecoder().decode(data as Uint8Array), oldname);
   },
 });
+
+Deno.test("[std/node/fs] readlink callback isn't called twice if error is thrown", async () => {
+  const importUrl = new URL("./_fs_readlink.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import { readlink } from ${JSON.stringify(importUrl)}`,
+    invocation: `readlink(${JSON.stringify(newname)}, `,
+  });
+});

std/node/_fs/_fs_realpath.ts

@@ -12,9 +12,10 @@ export function realpath(
   if (!callback) {
     throw new Error("No callback function supplied");
   }
-  Deno.realPath(path)
+  Deno.realPath(path).then(
-    .then((path) => callback!(null, path))
+    (path) => callback!(null, path),
-    .catch((err) => callback!(err));
+    (err) => callback!(err),
+  );
 }
 
 export function realpathSync(path: string): string {

std/node/_fs/_fs_realpath_test.ts

@@ -1,4 +1,6 @@
+import * as path from "../../path/mod.ts";
 import { assertEquals } from "../../testing/asserts.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import { realpath, realpathSync } from "./_fs_realpath.ts";
 
 Deno.test("realpath", async function () {
@@ -34,3 +36,19 @@ Deno.test("realpathSync", function () {
   const realSymLinkPath = realpathSync(tempFileAlias);
   assertEquals(realPath, realSymLinkPath);
 });
+
+Deno.test("[std/node/fs] realpath callback isn't called twice if error is thrown", async () => {
+  const tempDir = await Deno.makeTempDir();
+  const tempFile = path.join(tempDir, "file.txt");
+  const linkFile = path.join(tempDir, "link.txt");
+  await Deno.writeTextFile(tempFile, "hello world");
+  await Deno.symlink(tempFile, linkFile, { type: "file" });
+  const importUrl = new URL("./_fs_realpath.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import { realpath } from ${JSON.stringify(importUrl)}`,
+    invocation: `realpath(${JSON.stringify(`${tempDir}/link.txt`)}, `,
+    async cleanup() {
+      await Deno.remove(tempDir, { recursive: true });
+    },
+  });
+});

std/node/_fs/_fs_rename.ts

@@ -10,9 +10,7 @@ export function rename(
 
   if (!callback) throw new Error("No callback function supplied");
 
-  Deno.rename(oldPath, newPath)
+  Deno.rename(oldPath, newPath).then((_) => callback(), callback);
-    .then((_) => callback())
-    .catch(callback);
 }
 
 export function renameSync(oldPath: string | URL, newPath: string | URL) {

std/node/_fs/_fs_rename_test.ts

@@ -1,4 +1,5 @@
 import { assertEquals, fail } from "../../testing/asserts.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import { rename, renameSync } from "./_fs_rename.ts";
 import { existsSync } from "../../fs/mod.ts";
 import { join, parse } from "../../path/mod.ts";
@@ -17,8 +18,7 @@ Deno.test({
       .then(() => {
         assertEquals(existsSync(newPath), true);
         assertEquals(existsSync(file), false);
-      })
+      }, () => fail())
-      .catch(() => fail())
       .finally(() => {
         if (existsSync(file)) Deno.removeSync(file);
         if (existsSync(newPath)) Deno.removeSync(newPath);
@@ -36,3 +36,16 @@ Deno.test({
     assertEquals(existsSync(file), false);
   },
 });
+
+Deno.test("[std/node/fs] rename callback isn't called twice if error is thrown", async () => {
+  const tempFile = await Deno.makeTempFile();
+  const importUrl = new URL("./_fs_rename.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import { rename } from ${JSON.stringify(importUrl)}`,
+    invocation: `rename(${JSON.stringify(tempFile)}, 
+                        ${JSON.stringify(`${tempFile}.newname`)}, `,
+    async cleanup() {
+      await Deno.remove(`${tempFile}.newname`);
+    },
+  });
+});

std/node/_fs/_fs_rmdir.ts

@@ -27,8 +27,7 @@ export function rmdir(
   if (!callback) throw new Error("No callback function supplied");
 
   Deno.remove(path, { recursive: options?.recursive })
-    .then((_) => callback())
+    .then((_) => callback(), callback);
-    .catch(callback);
 }
 
 export function rmdirSync(path: string | URL, options?: rmdirOptions) {

std/node/_fs/_fs_rmdir_test.ts

@@ -3,6 +3,7 @@ import { rmdir, rmdirSync } from "./_fs_rmdir.ts";
 import { closeSync } from "./_fs_close.ts";
 import { existsSync } from "../../fs/mod.ts";
 import { join } from "../../path/mod.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 
 Deno.test({
   name: "ASYNC: removing empty folder",
@@ -14,8 +15,7 @@ Deno.test({
         resolve();
       });
     })
-      .then(() => assertEquals(existsSync(dir), false))
+      .then(() => assertEquals(existsSync(dir), false), () => fail())
-      .catch(() => fail())
       .finally(() => {
         if (existsSync(dir)) Deno.removeSync(dir);
       });
@@ -58,8 +58,7 @@ Deno.test({
         resolve();
       });
     })
-      .then(() => assertEquals(existsSync(dir), false))
+      .then(() => assertEquals(existsSync(dir), false), () => fail())
-      .catch(() => fail())
       .finally(() => {
         if (existsSync(dir)) Deno.removeSync(dir, { recursive: true });
         const rAfter = Deno.resources();
@@ -86,3 +85,16 @@ Deno.test({
   },
   ignore: Deno.build.os === "windows",
 });
+
+Deno.test("[std/node/fs] rmdir callback isn't called twice if error is thrown", async () => {
+  // The correct behaviour is not to catch any errors thrown,
+  // but that means there'll be an uncaught error and the test will fail.
+  // So the only way to test this is to spawn a subprocess, and succeed if it has a non-zero exit code.
+  // (assertThrowsAsync won't work because there's no way to catch the error.)
+  const tempDir = await Deno.makeTempDir();
+  const importUrl = new URL("./_fs_rmdir.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import { rmdir } from ${JSON.stringify(importUrl)}`,
+    invocation: `rmdir(${JSON.stringify(tempDir)}, `,
+  });
+});

std/node/_fs/_fs_stat.ts

@@ -234,11 +234,11 @@ export function CFISBIS(fileInfo: Deno.FileInfo, bigInt: boolean) {
 }
 
 export type statCallbackBigInt = (
-  err: Error | undefined,
+  err: Error | null,
   stat: BigIntStats,
 ) => void;
 
-export type statCallback = (err: Error | undefined, stat: Stats) => void;
+export type statCallback = (err: Error | null, stat: Stats) => void;
 
 export function stat(path: string | URL, callback: statCallback): void;
 export function stat(
@@ -260,8 +260,7 @@ export function stat(
     (typeof optionsOrCallback === "function"
       ? optionsOrCallback
       : maybeCallback) as (
-        err: Error | undefined,
+        ...args: [Error] | [null, BigIntStats | Stats]
-        stat: BigIntStats | Stats,
       ) => void;
   const options = typeof optionsOrCallback === "object"
     ? optionsOrCallback
@@ -269,9 +268,10 @@ export function stat(
 
   if (!callback) throw new Error("No callback function supplied");
 
-  Deno.stat(path)
+  Deno.stat(path).then(
-    .then((stat) => callback(undefined, CFISBIS(stat, options.bigint)))
+    (stat) => callback(null, CFISBIS(stat, options.bigint)),
-    .catch((err) => callback(err, err));
+    (err) => callback(err),
+  );
 }
 
 export function statSync(path: string | URL): Stats;

std/node/_fs/_fs_stat_test.ts

@@ -1,3 +1,4 @@
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import { BigIntStats, stat, Stats, statSync } from "./_fs_stat.ts";
 import { assertEquals, fail } from "../../testing/asserts.ts";
 
@@ -68,8 +69,7 @@ Deno.test({
         resolve(stat);
       });
     })
-      .then((stat) => assertStats(stat, Deno.statSync(file)))
+      .then((stat) => assertStats(stat, Deno.statSync(file)), () => fail())
-      .catch(() => fail())
       .finally(() => Deno.removeSync(file));
   },
 });
@@ -92,8 +92,10 @@ Deno.test({
         resolve(stat);
       });
     })
-      .then((stat) => assertStatsBigInt(stat, Deno.statSync(file)))
+      .then(
-      .catch(() => fail())
+        (stat) => assertStatsBigInt(stat, Deno.statSync(file)),
+        () => fail(),
+      )
       .finally(() => Deno.removeSync(file));
   },
 });
@@ -105,3 +107,15 @@ Deno.test({
     assertStatsBigInt(statSync(file, { bigint: true }), Deno.statSync(file));
   },
 });
+
+Deno.test("[std/node/fs] stat callback isn't called twice if error is thrown", async () => {
+  const tempFile = await Deno.makeTempFile();
+  const importUrl = new URL("./_fs_stat.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import { stat } from ${JSON.stringify(importUrl)}`,
+    invocation: `stat(${JSON.stringify(tempFile)}, `,
+    async cleanup() {
+      await Deno.remove(tempFile);
+    },
+  });
+});

std/node/_fs/_fs_unlink.ts

@@ -1,8 +1,6 @@
 export function unlink(path: string | URL, callback: (err?: Error) => void) {
   if (!callback) throw new Error("No callback function supplied");
-  Deno.remove(path)
+  Deno.remove(path).then((_) => callback(), callback);
-    .then((_) => callback())
-    .catch(callback);
 }
 
 export function unlinkSync(path: string | URL) {

std/node/_fs/_fs_unlink_test.ts

@@ -1,5 +1,6 @@
 import { assertEquals, fail } from "../../testing/asserts.ts";
 import { existsSync } from "../../fs/mod.ts";
+import { assertCallbackErrorUncaught } from "../_utils.ts";
 import { unlink, unlinkSync } from "./_fs_unlink.ts";
 
 Deno.test({
@@ -12,8 +13,7 @@ Deno.test({
         resolve();
       });
     })
-      .then(() => assertEquals(existsSync(file), false))
+      .then(() => assertEquals(existsSync(file), false), () => fail())
-      .catch(() => fail())
       .finally(() => {
         if (existsSync(file)) Deno.removeSync(file);
       });
@@ -28,3 +28,12 @@ Deno.test({
     assertEquals(existsSync(file), false);
   },
 });
+
+Deno.test("[std/node/fs] unlink callback isn't called twice if error is thrown", async () => {
+  const tempFile = await Deno.makeTempFile();
+  const importUrl = new URL("./_fs_unlink.ts", import.meta.url);
+  await assertCallbackErrorUncaught({
+    prelude: `import { unlink } from ${JSON.stringify(importUrl)}`,
+    invocation: `unlink(${JSON.stringify(tempFile)}, `,
+  });
+});

std/node/_fs/_fs_watch_test.ts

@@ -12,21 +12,15 @@ Deno.test({
   async fn() {
     const file = Deno.makeTempFileSync();
     const result: Array<[string, string]> = [];
-    await new Promise((resolve) => {
     const watcher = watch(
       file,
       (eventType, filename) => result.push([eventType, filename]),
     );
-      wait(100)
+    await wait(100);
-        .then(() => Deno.writeTextFileSync(file, "something"))
+    Deno.writeTextFileSync(file, "something");
-        .then(() => wait(100))
+    await wait(100);
-        .then(() => watcher.close())
+    watcher.close();
-        .then(() => wait(100))
+    await wait(100);
-        .then(resolve);
-    })
-      .then(() => {
     assertEquals(result.length >= 1, true);
-      })
-      .catch(() => fail());
   },
 });

std/node/_utils.ts

@@ -1,6 +1,6 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
 import { deferred } from "../async/mod.ts";
-import { fail } from "../testing/asserts.ts";
+import { assert, assertStringIncludes, fail } from "../testing/asserts.ts";
 
 export type BinaryEncodings = "binary";
 
@@ -37,26 +37,28 @@ export type MaybeEmpty<T> = T | null | undefined;
 export function intoCallbackAPI<T>(
   // deno-lint-ignore no-explicit-any
   func: (...args: any[]) => Promise<T>,
-  cb: MaybeEmpty<(err: MaybeNull<Error>, value: MaybeEmpty<T>) => void>,
+  cb: MaybeEmpty<(err: MaybeNull<Error>, value?: MaybeEmpty<T>) => void>,
   // deno-lint-ignore no-explicit-any
   ...args: any[]
 ): void {
-  func(...args)
+  func(...args).then(
-    .then((value) => cb && cb(null, value))
+    (value) => cb && cb(null, value),
-    .catch((err) => cb && cb(err, null));
+    (err) => cb && cb(err),
+  );
 }
 
 export function intoCallbackAPIWithIntercept<T1, T2>(
   // deno-lint-ignore no-explicit-any
   func: (...args: any[]) => Promise<T1>,
   interceptor: (v: T1) => T2,
-  cb: MaybeEmpty<(err: MaybeNull<Error>, value: MaybeEmpty<T2>) => void>,
+  cb: MaybeEmpty<(err: MaybeNull<Error>, value?: MaybeEmpty<T2>) => void>,
   // deno-lint-ignore no-explicit-any
   ...args: any[]
 ): void {
-  func(...args)
+  func(...args).then(
-    .then((value) => cb && cb(null, interceptor(value)))
+    (value) => cb && cb(null, interceptor(value)),
-    .catch((err) => cb && cb(err, null));
+    (err) => cb && cb(err),
+  );
 }
 
 export function spliceOne(list: string[], index: number): void {
@@ -203,3 +205,43 @@ export function mustCall<T extends unknown[]>(
     callback,
   ];
 }
+/** Asserts that an error thrown in a callback will not be wrongly caught. */
+export async function assertCallbackErrorUncaught(
+  { prelude, invocation, cleanup }: {
+    /** Any code which needs to run before the actual invocation (notably, any import statements). */
+    prelude?: string;
+    /** 
+     * The start of the invocation of the function, e.g. `open("foo.txt", `.
+     * The callback will be added after it. 
+     */
+    invocation: string;
+    /** Called after the subprocess is finished but before running the assertions, e.g. to clean up created files. */
+    cleanup?: () => Promise<void> | void;
+  },
+) {
+  // Since the error has to be uncaught, and that will kill the Deno process,
+  // the only way to test this is to spawn a subprocess.
+  const p = Deno.run({
+    cmd: [
+      Deno.execPath(),
+      "eval",
+      "--no-check", // Running TSC for every one of these tests would take way too long
+      "--unstable",
+      `${prelude ?? ""}
+
+      ${invocation}(err) => {
+        // If the bug is present and the callback is called again with an error,
+        // don't throw another error, so if the subprocess fails we know it had the correct behaviour.
+        if (!err) throw new Error("success");
+      });`,
+    ],
+    stderr: "piped",
+  });
+  const status = await p.status();
+  const stderr = new TextDecoder().decode(await Deno.readAll(p.stderr));
+  p.close();
+  p.stderr.close();
+  await cleanup?.();
+  assert(!status.success);
+  assertStringIncludes(stderr, "Error: success");
+}