fix(node/fs): chmod function throws unnecessary TypeError on Windows (#…

…3168)
denoland · Feb 8, 2023 · 62ad636 · 62ad636
1 parent 62af6ef
commit 62ad636
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 2 deletions.
diff --git a/node/_fs/_fs_chmod.ts b/node/_fs/_fs_chmod.ts
@@ -12,7 +12,18 @@ export function chmod(
   callback: CallbackWithError,
 ) {
   path = getValidatedPath(path).toString();
-  mode = parseFileMode(mode, "mode");
+
+  try {
+    mode = parseFileMode(mode, "mode");
+  } catch (error) {
+    // TODO(PolarETech): Errors should not be ignored when Deno.chmod is supported on Windows.
+    // https://github.com/denoland/deno_std/issues/2916
+    if (Deno.build.os === "windows") {
+      mode = 0; // set dummy value to avoid type checking error at Deno.chmod
+    } else {
+      throw error;
+    }
+  }
 
   Deno.chmod(pathModule.toNamespacedPath(path), mode).catch((error) => {
     // Ignore NotSupportedError that occurs on windows
@@ -33,7 +44,18 @@ export const chmodPromise = promisify(chmod) as (
 
 export function chmodSync(path: string | URL, mode: string | number) {
   path = getValidatedPath(path).toString();
-  mode = parseFileMode(mode, "mode");
+
+  try {
+    mode = parseFileMode(mode, "mode");
+  } catch (error) {
+    // TODO(PolarETech): Errors should not be ignored when Deno.chmodSync is supported on Windows.
+    // https://github.com/denoland/deno_std/issues/2916
+    if (Deno.build.os === "windows") {
+      mode = 0; // set dummy value to avoid type checking error at Deno.chmodSync
+    } else {
+      throw error;
+    }
+  }
 
   try {
     Deno.chmodSync(pathModule.toNamespacedPath(path), mode);

diff --git a/node/_fs/_fs_chmod_test.ts b/node/_fs/_fs_chmod_test.ts
@@ -34,6 +34,23 @@ Deno.test({
   },
 });
 
+Deno.test({
+  name: "ASYNC: don't throw errors for mode parameter (Windows)",
+  ignore: !isWindows,
+  async fn() {
+    const tempFile: string = await Deno.makeTempFile();
+    await new Promise<void>((resolve, reject) => {
+      // @ts-ignore for test
+      chmod(tempFile, null, (err) => {
+        if (err) reject(err);
+        else resolve();
+      });
+    }).finally(() => {
+      Deno.removeSync(tempFile);
+    });
+  },
+});
+
 Deno.test({
   name: "ASYNC: don't throw NotSupportedError (Windows)",
   ignore: !isWindows,
@@ -83,6 +100,20 @@ Deno.test({
   },
 });
 
+Deno.test({
+  name: "SYNC: don't throw errors for mode parameter (Windows)",
+  ignore: !isWindows,
+  fn() {
+    const tempFile: string = Deno.makeTempFileSync();
+    try {
+      // @ts-ignore for test
+      chmodSync(tempFile, null);
+    } finally {
+      Deno.removeSync(tempFile);
+    }
+  },
+});
+
 Deno.test({
   name: "SYNC: don't throw NotSupportedError (Windows)",
   ignore: !isWindows,