Repository: denoland/deno
Pull request: fix(ext/node): don't throw error on invalid path input on `exists` and `existsSync`

Currently `fs.exists` and `fs.existsSync` throws error when invalid path is given to the input.

The expected behavior is to:
- Call the callback with false on `fs.exists`.
- Return false on `fs.existsSync`.

This PR addresses that, the prefer-primordials lint rule, and other small changes changes that allow these tests to pass:
- [parallel/test-fs-exists.js](https://github.com/denoland/node_test/blob/8846b5392f4ebd3d4e995b9bed61c23c0192d806/test/parallel/test-fs-exists.js)
- [parallel/test-fs-existssync-false.js](https://github.com/denoland/node_test/blob/8846b5392f4ebd3d4e995b9bed61c23c0192d806/test/parallel/test-fs-existssync-false.js)

Towards #29972, #24236

Changed files:
filename: ext/node/polyfills/_fs/_fs_common.ts
patch: @@ -253,9 +253,9 @@ export function maybeCallback(cb: unknown) {
 // Ensure that callbacks run in the global context. Only use this function
 // for callbacks that are passed to the binding layer, callbacks that are
 // invoked from JS already run in the proper scope.
-export function makeCallback(
+export function makeCallback<T>(
   this: unknown,
-  cb?: (err: Error | null, result?: unknown) => void,
+  cb?: (arg: T) => void,
 ) {
   validateFunction(cb, "cb");
 

filename: ext/node/polyfills/_fs/_fs_exists.ts
patch: @@ -1,41 +1,56 @@
 // Copyright 2018-2025 the Deno authors. MIT license.
 
-// TODO(petamoriken): enable prefer-primordials for node polyfills
-// deno-lint-ignore-file prefer-primordials
-
 import { op_node_fs_exists, op_node_fs_exists_sync } from "ext:core/ops";
+import { getValidatedPathToString } from "ext:deno_node/internal/fs/utils.mjs";
+import { primordials } from "ext:core/mod.js";
+import { makeCallback } from "ext:deno_node/_fs/_fs_common.ts";
+import type { Buffer } from "node:buffer";
+import * as pathModule from "node:path";
+import { kCustomPromisifiedSymbol } from "ext:deno_node/internal/util.mjs";
 
-import { pathFromURL } from "ext:deno_web/00_infra.js";
+const { ObjectDefineProperty, Promise, PromisePrototypeThen } = primordials;
 
 type ExistsCallback = (exists: boolean) => void;
 
 /**
- * TODO: Also accept 'path' parameter as a Node polyfill Buffer type once these
- * are implemented. See https://github.com/denoland/deno/issues/3403
  * Deprecated in node api
  */
-export function exists(path: string | URL, callback: ExistsCallback) {
-  path = path instanceof URL ? pathFromURL(path) : path;
-  op_node_fs_exists(path).then(callback);
+export function exists(path: string | Buffer | URL, callback: ExistsCallback) {
+  callback = makeCallback(callback);
+
+  try {
+    path = getValidatedPathToString(path);
+  } catch {
+    callback(false);
+    return;
+  }
+
+  PromisePrototypeThen(
+    op_node_fs_exists(pathModule.toNamespacedPath(path)),
+    callback,
+  );
 }
 
 // The callback of fs.exists doesn't have standard callback signature.
 // We need to provide special implementation for promisify.
 // See https://github.com/nodejs/node/pull/13316
-const kCustomPromisifiedSymbol = Symbol.for("nodejs.util.promisify.custom");
-Object.defineProperty(exists, kCustomPromisifiedSymbol, {
+ObjectDefineProperty(exists, kCustomPromisifiedSymbol, {
+  __proto__: null,
   value: (path: string | URL) => {
     return new Promise((resolve) => {
       exists(path, (exists) => resolve(exists));
     });
   },
+  enumerable: false,
+  writable: false,
+  configurable: true,
 });
 
-/**
- * TODO: Also accept 'path' parameter as a Node polyfill Buffer or URL type once these
- * are implemented. See https://github.com/denoland/deno/issues/3403
- */
-export function existsSync(path: string | URL): boolean {
-  path = path instanceof URL ? pathFromURL(path) : path;
-  return op_node_fs_exists_sync(path);
+export function existsSync(path: string | Buffer | URL): boolean {
+  try {
+    path = getValidatedPathToString(path);
+  } catch {
+    return false;
+  }
+  return op_node_fs_exists_sync(pathModule.toNamespacedPath(path));
 }

filename: ext/node/polyfills/internal/util.mjs
patch: @@ -46,7 +46,9 @@ export function once(callback) {
 // In addition to being accessible through util.promisify.custom,
 // this symbol is registered globally and can be accessed in any environment as
 // Symbol.for('nodejs.util.promisify.custom').
-const kCustomPromisifiedSymbol = SymbolFor("nodejs.util.promisify.custom");
+export const kCustomPromisifiedSymbol = SymbolFor(
+  "nodejs.util.promisify.custom",
+);
 // This is an internal Node symbol used by functions returning multiple
 // arguments, e.g. ['bytesRead', 'buffer'] for fs.read().
 const kCustomPromisifyArgsSymbol = SymbolFor(

filename: tests/node_compat/config.toml
patch: @@ -424,6 +424,8 @@
 "parallel/test-fs-close.js" = {}
 "parallel/test-fs-constants.js" = {}
 "parallel/test-fs-empty-readStream.js" = {}
+"parallel/test-fs-exists.js" = {}
+"parallel/test-fs-existssync-false.js" = {}
 "parallel/test-fs-fchown.js" = {}
 "parallel/test-fs-long-path.js" = {}
 "parallel/test-fs-promises-exists.js" = {}