denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
96.93k stars 5.35k forks source link

node-compa: better error message #25921

Open familyboat opened 4 weeks ago

familyboat commented 4 weeks ago

Version: Deno 1.46.1


// main.ts
import fs from "node:fs";

/**
 * read file with node compa
 */
function readFileWithNode(file: string) {
  try {
    fs.statSync(file);
  } catch (error: unknown) {
    // No path information in error
    console.log(`Error occurred while reading file with node: ${error}`);
  }
}

/**
 * read file with deno
 */
function readFileWithDeno(file: string) {
  try {
    Deno.statSync(file);
  } catch (error: unknown) {
    // Has path information in error
    console.log(`Error occurred while reading file with deno: ${error}`);
  }
}

readFileWithNode("non-exist-file");
readFileWithDeno("non-exist-file");

Run below command:

deno run -A main.ts

The output is:

Error occurred while reading file with node: Error: ENOENT: no such file or directory, stat
Error occurred while reading file with deno: NotFound: 系统找不到指定的文件。 (os error 2): stat 'non-exist-file'

In this case, better error message(has path information) is useful for debug.

lucacasonato commented 3 weeks ago

I think there is some error wrapping going on here that needs to just be removed from the node:fs code path.

familyboat commented 3 weeks ago

Do you think adding the missing file path information is sufficient for this case. If so, i can make a PR. @lucacasonato

familyboat commented 3 weeks ago

It is my first time to contribute to deno. So i follow the instruction in here. After everything set up, run cargo build -vv. It exited in the fetching rust_v8 precompile stage(by the way, i am in China). So i can not make a PR. If i am right, just addding path parameter in this line: https://github.com/denoland/deno/blob/f5caf9dd1b0f612e2a03e802d4dca41cb13a006b/ext/node/polyfills/_fs/_fs_stat.ts#L420 can fix this issue.

familyboat commented 3 weeks ago

By using local version of rusty_v8 precompile, deno was built successfully. I added the path parameter in here: https://github.com/denoland/deno/blob/f5caf9dd1b0f612e2a03e802d4dca41cb13a006b/ext/node/polyfills/_fs/_fs_stat.ts#L420

Change it to:

throw denoErrorToNodeError(err, { syscall: "stat", path: path.toString() });

Below is the test code:

import fs from "node:fs";

function readFileWithNode(file: string | URL) {
  const type = typeof file === "string" ? "string" : "url";

  try {
    fs.statSync(file);
  } catch (error: unknown) {
    console.log(
      `Error occurred while reading ${type} file with node: ${error}`,
    );
  }
}

function readFileWithDeno(file: string | URL) {
  const type = typeof file === "string" ? "string" : "url";

  try {
    Deno.statSync(file);
  } catch (error: unknown) {
    console.log(
      `Error occurred while reading ${type} file with deno: ${error}`,
    );
  }
}

const file = "non-exist-file";
const fileUrl = new URL(file, import.meta.url);
readFileWithNode(file);
readFileWithDeno(file);
readFileWithNode(fileUrl);
readFileWithDeno(fileUrl);

Output is:

Error occurred while reading string file with node: Error: ENOENT: no such file or directory, stat 'non-exist-file'
Error occurred while reading string file with deno: NotFound: No such file or directory (os error 2): stat 'non-exist-file'
Error occurred while reading url file with node: Error: ENOENT: no such file or directory, stat 'file:///root/test/node-file-error-message/non-exist-file'
Error occurred while reading url file with deno: NotFound: No such file or directory (os error 2): stat '/root/test/node-file-error-message/non-exist-file'

Do you think it is ok?