diff options
Diffstat (limited to 'std/node/_fs')
40 files changed, 429 insertions, 147 deletions
diff --git a/std/node/_fs/_fs_appendFile.ts b/std/node/_fs/_fs_appendFile.ts index fab72d0f7..ee01b9cc8 100644 --- a/std/node/_fs/_fs_appendFile.ts +++ b/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) - .catch(reject); + .then(resolve, reject); } }) .then(() => { closeRidIfNecessary(typeof pathOrRid === "string", rid); - callbackFn(); - }) - .catch((err) => { + callbackFn(null); + }, (err) => { closeRidIfNecessary(typeof pathOrRid === "string", rid); callbackFn(err); }); diff --git a/std/node/_fs/_fs_appendFile_test.ts b/std/node/_fs/_fs_appendFile_test.ts index af430ec97..6a77974fd 100644 --- a/std/node/_fs/_fs_appendFile_test.ts +++ b/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"); - }) - .catch((err) => { + }, (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"); - }) - .catch((err) => { + }, (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); - }) - .catch((err) => { + }, (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); + }, + }); +}); diff --git a/std/node/_fs/_fs_chmod.ts b/std/node/_fs/_fs_chmod.ts index cb673da67..bf4031d2c 100644 --- a/std/node/_fs/_fs_chmod.ts +++ b/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)) - .then(() => callback()) - .catch(callback); + Deno.chmod(path, getResolvedMode(mode)).then(() => callback(null), callback); } /** diff --git a/std/node/_fs/_fs_chmod_test.ts b/std/node/_fs/_fs_chmod_test.ts index 5f946794e..4d8647782 100644 --- a/std/node/_fs/_fs_chmod_test.ts +++ b/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); + }, + }); + }, +}); diff --git a/std/node/_fs/_fs_chown.ts b/std/node/_fs/_fs_chown.ts index ca709f2ea..f9e7326ce 100644 --- a/std/node/_fs/_fs_chown.ts +++ b/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) - .then(() => callback()) - .catch(callback); + Deno.chown(path, uid, gid).then(() => callback(null), callback); } /** diff --git a/std/node/_fs/_fs_chown_test.ts b/std/node/_fs/_fs_chown_test.ts index dc881c27a..8d74f657a 100644 --- a/std/node/_fs/_fs_chown_test.ts +++ b/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); + }, + }); + }, +}); diff --git a/std/node/_fs/_fs_close.ts b/std/node/_fs/_fs_close.ts index 8e9a1f620..721827421 100644 --- a/std/node/_fs/_fs_close.ts +++ b/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 { diff --git a/std/node/_fs/_fs_close_test.ts b/std/node/_fs/_fs_close_test.ts index 047f6d977..df5a93240 100644 --- a/std/node/_fs/_fs_close_test.ts +++ b/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); + }, + }); +}); diff --git a/std/node/_fs/_fs_common.ts b/std/node/_fs/_fs_common.ts index 73a0a81ef..c233b2d76 100644 --- a/std/node/_fs/_fs_common.ts +++ b/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; diff --git a/std/node/_fs/_fs_copy.ts b/std/node/_fs/_fs_copy.ts index 00d17798f..3242b97b3 100644 --- a/std/node/_fs/_fs_copy.ts +++ b/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) - .then(() => callback()) - .catch(callback); + Deno.copyFile(source, destination).then(() => callback(null), callback); } export function copyFileSync(source: string | URL, destination: string): void { diff --git a/std/node/_fs/_fs_copy_test.ts b/std/node/_fs/_fs_copy_test.ts index c61fffb14..4fdf78cb0 100644 --- a/std/node/_fs/_fs_copy_test.ts +++ b/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 }); + }, + }); +}); diff --git a/std/node/_fs/_fs_dir.ts b/std/node/_fs/_fs_dir.ts index b2353146b..4f7beec31 100644 --- a/std/node/_fs/_fs_dir.ts +++ b/std/node/_fs/_fs_dir.ts @@ -32,10 +32,9 @@ export default class Dir { if (callback) { callback(null, value ? value : null); } - }) - .catch((err) => { + }, (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) => { - try { - if (callback) { - callback(null); - } - resolve(); - } catch (err) { - if (callback) { - callback(err); - } - reject(err); + return new Promise((resolve) => { + if (callback) { + callback(null); } + resolve(); }); } diff --git a/std/node/_fs/_fs_dir_test.ts b/std/node/_fs/_fs_dir_test.ts index b2b0998b3..f12563fd5 100644 --- a/std/node/_fs/_fs_dir_test.ts +++ b/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); + }, + }); +}); diff --git a/std/node/_fs/_fs_exists.ts b/std/node/_fs/_fs_exists.ts index 91f2ce105..5cfa26a9d 100644 --- a/std/node/_fs/_fs_exists.ts +++ b/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) - .then(() => { - callback(true); - }) - .catch(() => callback(false)); + Deno.lstat(path).then(() => callback(true), () => callback(false)); } /** diff --git a/std/node/_fs/_fs_exists_test.ts b/std/node/_fs/_fs_exists_test.ts index 6e120af98..ab0ae5c15 100644 --- a/std/node/_fs/_fs_exists_test.ts +++ b/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"); +}); diff --git a/std/node/_fs/_fs_link.ts b/std/node/_fs/_fs_link.ts index 0fe7e1b4a..14a8189cb 100644 --- a/std/node/_fs/_fs_link.ts +++ b/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) - .then(() => callback()) - .catch(callback); + Deno.link(existingPath, newPath).then(() => callback(null), callback); } /** diff --git a/std/node/_fs/_fs_link_test.ts b/std/node/_fs/_fs_link_test.ts index 4804b2eed..9e2aeb8b6 100644 --- a/std/node/_fs/_fs_link_test.ts +++ b/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"); - }) - .catch((err) => { + }, (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 }); + }, + }); +}); diff --git a/std/node/_fs/_fs_lstat.ts b/std/node/_fs/_fs_lstat.ts index 0b79fb665..55624cd5e 100644 --- a/std/node/_fs/_fs_lstat.ts +++ b/std/node/_fs/_fs_lstat.ts @@ -27,8 +27,7 @@ export function lstat( (typeof optionsOrCallback === "function" ? optionsOrCallback : maybeCallback) as ( - err: Error | undefined, - stat: BigIntStats | Stats, + ...args: [Error] | [null, 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) - .then((stat) => callback(undefined, CFISBIS(stat, options.bigint))) - .catch((err) => callback(err, err)); + Deno.lstat(path).then( + (stat) => callback(null, CFISBIS(stat, options.bigint)), + (err) => callback(err), + ); } export function lstatSync(path: string | URL): Stats; diff --git a/std/node/_fs/_fs_lstat_test.ts b/std/node/_fs/_fs_lstat_test.ts index 1da0a562f..b8c562538 100644 --- a/std/node/_fs/_fs_lstat_test.ts +++ b/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)); - }) - .catch(() => fail()) + }, () => fail()) .finally(() => { Deno.removeSync(file); }); @@ -41,8 +41,10 @@ Deno.test({ resolve(stat); }); }) - .then((stat) => assertStatsBigInt(stat, Deno.lstatSync(file))) - .catch(() => fail()) + .then( + (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); + }, + }); +}); diff --git a/std/node/_fs/_fs_mkdir.ts b/std/node/_fs/_fs_mkdir.ts index 3a060ee76..94cc24a42 100644 --- a/std/node/_fs/_fs_mkdir.ts +++ b/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); } - }) - .catch((err) => { + }, (err) => { if (typeof callback === "function") { callback(err); } diff --git a/std/node/_fs/_fs_mkdir_test.ts b/std/node/_fs/_fs_mkdir_test.ts index e3d68ea9c..c13d065fe 100644 --- a/std/node/_fs/_fs_mkdir_test.ts +++ b/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 }); + }, + }); +}); diff --git a/std/node/_fs/_fs_mkdtemp.ts b/std/node/_fs/_fs_mkdtemp.ts index a249f1911..226399aa4 100644 --- a/std/node/_fs/_fs_mkdtemp.ts +++ b/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)); }, ); } diff --git a/std/node/_fs/_fs_open.ts b/std/node/_fs/_fs_open.ts index bf53115de..55ecbdc1e 100644 --- a/std/node/_fs/_fs_open.ts +++ b/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); - callback(undefined, res); + res = openSync(path, flags, mode); } 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)) - .then((file) => callback(undefined, file.rid)) - .catch((err) => callback(err, err)); + Deno.open(path, convertFlagAndModeToOptions(flags, mode)).then( + (file) => callback(null, file.rid), + (err) => (callback as (err: Error) => void)(err), + ); } } diff --git a/std/node/_fs/_fs_open_test.ts b/std/node/_fs/_fs_open_test.ts index 16f60110e..f32a8ce0a 100644 --- a/std/node/_fs/_fs_open_test.ts +++ b/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}`); - }) - .catch(() => fail()) + }, () => 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); + }, + }); +}); diff --git a/std/node/_fs/_fs_readFile.ts b/std/node/_fs/_fs_readFile.ts index 7540c8207..4ad763e25 100644 --- a/std/node/_fs/_fs_readFile.ts +++ b/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)); } } diff --git a/std/node/_fs/_fs_readFile_test.ts b/std/node/_fs/_fs_readFile_test.ts index 50fe569ff..7af32c8a1 100644 --- a/std/node/_fs/_fs_readFile_test.ts +++ b/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); + }, + }); +}); diff --git a/std/node/_fs/_fs_readdir.ts b/std/node/_fs/_fs_readdir.ts index 9034eccf8..e36bf5ecf 100644 --- a/std/node/_fs/_fs_readdir.ts +++ b/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, - files: string[] | Dirent[] | Array<string | Dirent>, + ...args: [Error] | [null, 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); } } diff --git a/std/node/_fs/_fs_readdir_test.ts b/std/node/_fs/_fs_readdir_test.ts index 169e10245..165cb8141 100644 --- a/std/node/_fs/_fs_readdir_test.ts +++ b/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, [])) - .catch(() => fail()) + .then((files) => assertEquals(files, []), () => fail()) .finally(() => Deno.removeSync(dir)); }, }); @@ -40,10 +40,14 @@ Deno.test({ resolve(files); }); }) - .then((files) => - assertEqualsArrayAnyOrder(files, ["file1.txt", "some_dir", "file2.txt"]) + .then( + (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); + }, + }); +}); diff --git a/std/node/_fs/_fs_readlink_test.ts b/std/node/_fs/_fs_readlink_test.ts index 06cd7e2dc..cb3dd25b4 100644 --- a/std/node/_fs/_fs_readlink_test.ts +++ b/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)}, `, + }); +}); diff --git a/std/node/_fs/_fs_realpath.ts b/std/node/_fs/_fs_realpath.ts index 586a87cfc..95e699d65 100644 --- a/std/node/_fs/_fs_realpath.ts +++ b/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) - .then((path) => callback!(null, path)) - .catch((err) => callback!(err)); + Deno.realPath(path).then( + (path) => callback!(null, path), + (err) => callback!(err), + ); } export function realpathSync(path: string): string { diff --git a/std/node/_fs/_fs_realpath_test.ts b/std/node/_fs/_fs_realpath_test.ts index 9ab0173c1..08eb3ef16 100644 --- a/std/node/_fs/_fs_realpath_test.ts +++ b/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 }); + }, + }); +}); diff --git a/std/node/_fs/_fs_rename.ts b/std/node/_fs/_fs_rename.ts index ee7c00977..b121eacf9 100644 --- a/std/node/_fs/_fs_rename.ts +++ b/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) - .then((_) => callback()) - .catch(callback); + Deno.rename(oldPath, newPath).then((_) => callback(), callback); } export function renameSync(oldPath: string | URL, newPath: string | URL) { diff --git a/std/node/_fs/_fs_rename_test.ts b/std/node/_fs/_fs_rename_test.ts index d0ba89644..e35e5282e 100644 --- a/std/node/_fs/_fs_rename_test.ts +++ b/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); - }) - .catch(() => fail()) + }, () => 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`); + }, + }); +}); diff --git a/std/node/_fs/_fs_rmdir.ts b/std/node/_fs/_fs_rmdir.ts index 2035a1e71..e82e696e3 100644 --- a/std/node/_fs/_fs_rmdir.ts +++ b/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()) - .catch(callback); + .then((_) => callback(), callback); } export function rmdirSync(path: string | URL, options?: rmdirOptions) { diff --git a/std/node/_fs/_fs_rmdir_test.ts b/std/node/_fs/_fs_rmdir_test.ts index 006bd7b9d..6f9c33274 100644 --- a/std/node/_fs/_fs_rmdir_test.ts +++ b/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)) - .catch(() => fail()) + .then(() => assertEquals(existsSync(dir), false), () => fail()) .finally(() => { if (existsSync(dir)) Deno.removeSync(dir); }); @@ -58,8 +58,7 @@ Deno.test({ resolve(); }); }) - .then(() => assertEquals(existsSync(dir), false)) - .catch(() => fail()) + .then(() => assertEquals(existsSync(dir), false), () => 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)}, `, + }); +}); diff --git a/std/node/_fs/_fs_stat.ts b/std/node/_fs/_fs_stat.ts index 3ca2a9918..4f7903d16 100644 --- a/std/node/_fs/_fs_stat.ts +++ b/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, - stat: BigIntStats | Stats, + ...args: [Error] | [null, 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) - .then((stat) => callback(undefined, CFISBIS(stat, options.bigint))) - .catch((err) => callback(err, err)); + Deno.stat(path).then( + (stat) => callback(null, CFISBIS(stat, options.bigint)), + (err) => callback(err), + ); } export function statSync(path: string | URL): Stats; diff --git a/std/node/_fs/_fs_stat_test.ts b/std/node/_fs/_fs_stat_test.ts index 3ba8d1cb2..e991985ea 100644 --- a/std/node/_fs/_fs_stat_test.ts +++ b/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))) - .catch(() => fail()) + .then((stat) => assertStats(stat, Deno.statSync(file)), () => fail()) .finally(() => Deno.removeSync(file)); }, }); @@ -92,8 +92,10 @@ Deno.test({ resolve(stat); }); }) - .then((stat) => assertStatsBigInt(stat, Deno.statSync(file))) - .catch(() => fail()) + .then( + (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); + }, + }); +}); diff --git a/std/node/_fs/_fs_unlink.ts b/std/node/_fs/_fs_unlink.ts index aba734fe1..7349bee46 100644 --- a/std/node/_fs/_fs_unlink.ts +++ b/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) - .then((_) => callback()) - .catch(callback); + Deno.remove(path).then((_) => callback(), callback); } export function unlinkSync(path: string | URL) { diff --git a/std/node/_fs/_fs_unlink_test.ts b/std/node/_fs/_fs_unlink_test.ts index 46cde60b1..5021b1c38 100644 --- a/std/node/_fs/_fs_unlink_test.ts +++ b/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)) - .catch(() => fail()) + .then(() => assertEquals(existsSync(file), false), () => 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)}, `, + }); +}); diff --git a/std/node/_fs/_fs_watch_test.ts b/std/node/_fs/_fs_watch_test.ts index e85b4c9bc..00fce4ffd 100644 --- a/std/node/_fs/_fs_watch_test.ts +++ b/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) - .then(() => Deno.writeTextFileSync(file, "something")) - .then(() => wait(100)) - .then(() => watcher.close()) - .then(() => wait(100)) - .then(resolve); - }) - .then(() => { - assertEquals(result.length >= 1, true); - }) - .catch(() => fail()); + const watcher = watch( + file, + (eventType, filename) => result.push([eventType, filename]), + ); + await wait(100); + Deno.writeTextFileSync(file, "something"); + await wait(100); + watcher.close(); + await wait(100); + assertEquals(result.length >= 1, true); }, }); |