-
Notifications
You must be signed in to change notification settings - Fork 29.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fs async functions (both callback and promises APIs) return Errors without stack trace #30944
Comments
This is sadly a known issue. Tracking the actual call site would be a significant impact for all I thought there would already be an issue for this but I failed finding it. |
There’s definitely a number of userland solutions for this issue, though, e.g. https://www.npmjs.com/package/trace. I wouldn’t mind bringing such an opt-in solution into core. |
@addaleax good point, we might just add a flag for that. |
can someone provide a summary of what the technical issue here is? |
@devsnek tracking the original call site for the async operation. The only way to really do that is (AFAIK) to create a stack trace for all calls. So it'll reduce the average performance for successful fs calls significantly. |
am i right in saying that the problem is that the error is created in c++ land while the js stack is empty? |
I believe we do not even attempt to create a proper error in that case. It would not point to the actual call site. |
@devsnek Yes, that is correct. The exception is created in I’m actually not sure anymore if long stack trace modules solve this. Fixing this might also require us to add better async support for these kinds of operations; the error is currently created outside of the async scope, so no async tracking is available. One thing that might be helpful for making async stack traces work better here is making |
What about the |
@heroboy If you're talking about the await optimization proposal, that's been implemented in Node.js v12.x and up. You can check if your node binary supports it like this:
Note that the optimization won't work if your program monkey-patches |
@bnoordhuis , No. I know current nodejs support this. And I know only async await functions have good support to async stack traces, many Promise functions will lost the stack.So if this issues fixed, will the |
There's no need to do anything on Node's side, if I understand your question correctly. It's all done inside V8. The one caveat is that it only works with |
I'm in a situation where I really need stack traces to debug an fs-intensive application (a build system), yet the available information about what is necessary to achieve fs tracing is vague. From this thread I understand the following:
The only ways to rectify the situation would then be to either change to sync fs calls or to wrap every call in a try-catch that throws its own error. Have I summed up the situation correctly? |
I tested the following code on node v16.13.1 on MacOS: // async.js
const fs = require("fs/promises");
process.on("uncaughtException", (e) => {
console.error(e.stack || e);
});
async function main() {
await fs.readFile("doesntexist");
}
main(); I have not yet discovered any usable environmental workaround. |
Is there a solution that doesn't change the code? |
I've gotten around this by wrapping the caught error in a new Error: const fs = require(`fs`);
try {
await fs.promises.readFile(`filePath`).catch((err)=>{
throw new Error(err);
});
} catch (error) {
console.error(error);
} Something like this should work too: fs.readFile(`filePath`, (err)=>{
if (err) throw new Error(err);
}); |
By the way this seems to be a common issue for other JS runtimes : denoland/deno#11859 oven-sh/bun#3972 but as noted in the Deno issue Chrome actually threads the information together in the DevTools console (when inspector is enabled), so I think at least we could do something similar. I already stepped into into the promise hooks code multiple times when I tried to debug the snippet in #49045 using inspector, so obviously we already have the information - and the overhead - in that case (can't make sense of the async hook stuff myself now, at least without some effort, maybe @nodejs/promises-debugging @nodejs/async_hooks would have better clues). |
This commit satisfies a few needs that we have in various projects: - When catching a throwable from some kind of operation, we want to be able to test whether the throwable is an error. - Furthermore, since the Error interface in TypeScript is pretty simple, we want to be able to test for different properties on an error (`code`, `stack`, etc.). - We want to wrap an error produced by a lower level part of the system with a different message, but preserve the original error using the `cause` property (note: this property was added in Node 18, so for older Nodes, we use the `pony-cause` library to set this). - Finally, we want to be able to take a throwable and produce an error that has a stacktrace. This is particularly useful for working with the `fs.promises` module, which (as of Node 22) [does not produce proper stacktraces][1]. [1]: nodejs/node#30944
Node's `fs.promises` module is somewhat difficult to work with because [the errors it produces lack proper stacktraces][1]. This commit attempts to address this issue by introducing wrapper functions around basic `fs.promises` operations which wrap caught errors to supply the missing stacktraces. It also introduces a couple of utility functions for reading and writing JSON files. Here are places where we currently use these functions (or something like them): - https://github.com/MetaMask/action-utils/blob/54ddd730746668cb4c1c88b4edfa720cbecf5e32/src/file-utils.ts - https://github.com/MetaMask/create-release-branch/blob/3556dee47163c921186051be7a1f3c98e2049db9/src/fs.ts One note about these functions is that, naturally, they require Node to use and will not work in the browser. Because we already create two kinds of bundles, one for CommonJS and another ESM, it would be difficult to create a second level of bundles, one for Node and another for the browser. Instead of requiring more complexity around the bundle configuration, this commit instead introduces another way to import the package. By default, you'll get all exports that are guaranteed to be cross-platform. That means that the file utilities won't show up: ``` typescript // ❌ import { readFile } from "@metamask/utils"; ``` If you want all of the cross-platform exports plus the Node-specific ones, you will have to import "@metamask/utils/node". For instance: ``` typescript // ✅ import { readFile } from "@metamask/utils/node"; ``` [1]: nodejs/node#30944
This commit adds several utility functions that are related to the filesystem in some way. - Most of the functions here are wrappers around basic filesystem operations with better error handling. Node's `fs.promises` module is somewhat difficult to work with because [the errors it produces lack proper stacktraces][1]. The functions that directly use `fs.promises` wrap caught errors in order to supply the missing stacktraces. - Two functions make it easier to read and write JSON files (with support for JSON-compatible interfaces, such as [JSON5][2]). - One function, `createSandbox`, is designed to wrap tests that need a temporary directory to work within, such as those for a command-line tool that makes changes to the filesystem. Here are places where we currently use these utilities (or something like them): - https://github.com/MetaMask/action-utils/blob/54ddd730746668cb4c1c88b4edfa720cbecf5e32/src/file-utils.ts - https://github.com/MetaMask/create-release-branch/blob/3556dee47163c921186051be7a1f3c98e2049db9/src/fs.ts - https://github.com/MetaMask/create-release-branch/blob/3556dee47163c921186051be7a1f3c98e2049db9/tests/helpers.ts One note about these utilities is that they require Node to use and will not work in the browser. Because we already create two kinds of bundles, one for CommonJS and another ESM, it would be difficult to create a second level of bundles, one for Node and another for the browser. Instead of requiring more complexity around the bundle configuration, this commit instead introduces another way to import the package. By default, you'll get all exports that are guaranteed to be cross-platform. That means that the file utilities won't show up: ``` typescript // ❌ import { readFile } from "@metamask/utils"; ``` If you want all of the cross-platform exports plus the Node-specific ones, you will have to import "@metamask/utils/node". For instance: ``` typescript // ✅ import { readFile } from "@metamask/utils/node"; ``` [1]: nodejs/node#30944 [2]: https://www.npmjs.com/package/json5
This commit adds several utility functions that are related to the filesystem in some way. - Most of the functions here are wrappers around basic filesystem operations with better error handling. Node's `fs.promises` module is somewhat difficult to work with because [the errors it produces lack proper stacktraces][1]. The functions that directly use `fs.promises` wrap caught errors in order to supply the missing stacktraces. - Two functions make it easier to read and write JSON files (with support for JSON-compatible interfaces, such as [JSON5][2]). - One function, `createSandbox`, is designed to wrap tests that need a temporary directory to work within, such as those for a command-line tool that makes changes to the filesystem. Here are places where we currently use these utilities (or something like them): - https://github.com/MetaMask/action-utils/blob/54ddd730746668cb4c1c88b4edfa720cbecf5e32/src/file-utils.ts - https://github.com/MetaMask/create-release-branch/blob/3556dee47163c921186051be7a1f3c98e2049db9/src/fs.ts - https://github.com/MetaMask/create-release-branch/blob/3556dee47163c921186051be7a1f3c98e2049db9/tests/helpers.ts One note about these utilities is that they require Node to use and will not work in the browser. Because we already create two kinds of bundles, one for CommonJS and another ESM, it would be difficult to create a second level of bundles, one for Node and another for the browser. Instead of requiring more complexity around the bundle configuration, this commit instead introduces another way to import the package. By default, you'll get all exports that are guaranteed to be cross-platform. That means that the file utilities won't show up: ``` typescript // ❌ import { readFile } from "@metamask/utils"; ``` If you want all of the cross-platform exports plus the Node-specific ones, you will have to import "@metamask/utils/node". For instance: ``` typescript // ✅ import { readFile } from "@metamask/utils/node"; ``` [1]: nodejs/node#30944 [2]: https://www.npmjs.com/package/json5
This commit adds several utility functions that are related to the filesystem in some way. - Most of the functions here are wrappers around basic filesystem operations with better error handling. Node's `fs.promises` module is somewhat difficult to work with because [the errors it produces lack proper stacktraces][1]. The functions that directly use `fs.promises` wrap caught errors in order to supply the missing stacktraces. - Two functions make it easier to read and write JSON files (with support for JSON-compatible interfaces, such as [JSON5][2]). - One function, `createSandbox`, is designed to wrap tests that need a temporary directory to work within, such as those for a command-line tool that makes changes to the filesystem. Here are places where we currently use these utilities (or something like them): - https://github.com/MetaMask/action-utils/blob/54ddd730746668cb4c1c88b4edfa720cbecf5e32/src/file-utils.ts - https://github.com/MetaMask/create-release-branch/blob/3556dee47163c921186051be7a1f3c98e2049db9/src/fs.ts - https://github.com/MetaMask/create-release-branch/blob/3556dee47163c921186051be7a1f3c98e2049db9/tests/helpers.ts One note about these utilities is that they require Node to use and will not work in the browser. Because we already create two kinds of bundles, one for CommonJS and another ESM, it would be difficult to create a second level of bundles, one for Node and another for the browser. Instead of requiring more complexity around the bundle configuration, this commit instead introduces another way to import the package. By default, you'll get all exports that are guaranteed to be cross-platform. That means that the file utilities won't show up: ``` typescript // ❌ import { readFile } from "@metamask/utils"; ``` If you want all of the cross-platform exports plus the Node-specific ones, you will have to import "@metamask/utils/node". For instance: ``` typescript // ✅ import { readFile } from "@metamask/utils/node"; ``` [1]: nodejs/node#30944 [2]: https://www.npmjs.com/package/json5
This commit satisfies a few needs that we have in various projects: - When catching a throwable from some kind of operation, we want to be able to test whether the throwable is an error. - Furthermore, since the Error interface in TypeScript is pretty simple, we want to be able to test for different properties on an error (`code`, `stack`, etc.). - We want to wrap an error produced by a lower level part of the system with a different message, but preserve the original error using the `cause` property (note: this property was added in Node 18, so for older Nodes, we use the `pony-cause` library to set this). - We want to be able to take a throwable and produce an error that has a stacktrace. This is particularly useful for working with the `fs.promises` module, which (as of Node 22) [does not produce proper stacktraces][1]. - We want to be able to get a message from a throwable. [1]: nodejs/node#30944 --------- Co-authored-by: Jongsun Suh <[email protected]> Co-authored-by: legobeat <[email protected]>
This commit adds several utility functions that are related to the filesystem in some way. - Most of the functions here are wrappers around basic filesystem operations with better error handling. Node's `fs.promises` module is somewhat difficult to work with because [the errors it produces lack proper stacktraces][1]. The functions that directly use `fs.promises` wrap caught errors in order to supply the missing stacktraces. - Two functions make it easier to read and write JSON files (with support for JSON-compatible interfaces, such as [JSON5][2]). - One function, `createSandbox`, is designed to wrap tests that need a temporary directory to work within, such as those for a command-line tool that makes changes to the filesystem. Here are places where we currently use these utilities (or something like them): - https://github.com/MetaMask/action-utils/blob/54ddd730746668cb4c1c88b4edfa720cbecf5e32/src/file-utils.ts - https://github.com/MetaMask/create-release-branch/blob/3556dee47163c921186051be7a1f3c98e2049db9/src/fs.ts - https://github.com/MetaMask/create-release-branch/blob/3556dee47163c921186051be7a1f3c98e2049db9/tests/helpers.ts One note about these utilities is that they require Node to use and will not work in the browser. Because we already create two kinds of bundles, one for CommonJS and another ESM, it would be difficult to create a second level of bundles, one for Node and another for the browser. Instead of requiring more complexity around the bundle configuration, this commit instead introduces another way to import the package. By default, you'll get all exports that are guaranteed to be cross-platform. That means that the file utilities won't show up: ``` typescript // ❌ import { readFile } from "@metamask/utils"; ``` If you want all of the cross-platform exports plus the Node-specific ones, you will have to import "@metamask/utils/node". For instance: ``` typescript // ✅ import { readFile } from "@metamask/utils/node"; ``` [1]: nodejs/node#30944 [2]: https://www.npmjs.com/package/json5
This commit adds several utility functions that are related to the filesystem in some way. - Most of the functions here are wrappers around basic filesystem operations with better error handling. Node's `fs.promises` module is somewhat difficult to work with because [the errors it produces lack proper stacktraces][1]. The functions that directly use `fs.promises` wrap caught errors in order to supply the missing stacktraces. - Two functions make it easier to read and write JSON files (with support for JSON-compatible interfaces, such as [JSON5][2]). - One function, `createSandbox`, is designed to wrap tests that need a temporary directory to work within, such as those for a command-line tool that makes changes to the filesystem. Here are places where we currently use these utilities (or something like them): - https://github.com/MetaMask/action-utils/blob/54ddd730746668cb4c1c88b4edfa720cbecf5e32/src/file-utils.ts - https://github.com/MetaMask/create-release-branch/blob/3556dee47163c921186051be7a1f3c98e2049db9/src/fs.ts - https://github.com/MetaMask/create-release-branch/blob/3556dee47163c921186051be7a1f3c98e2049db9/tests/helpers.ts One note about these utilities is that they require Node to use and will not work in the browser. Because we already create two kinds of bundles, one for CommonJS and another ESM, it would be difficult to create a second level of bundles, one for Node and another for the browser. Instead of requiring more complexity around the bundle configuration, this commit instead introduces another way to import the package. By default, you'll get all exports that are guaranteed to be cross-platform. That means that the file utilities won't show up: ``` typescript // ❌ import { readFile } from "@metamask/utils"; ``` If you want all of the cross-platform exports plus the Node-specific ones, you will have to import "@metamask/utils/node". For instance: ``` typescript // ✅ import { readFile } from "@metamask/utils/node"; ``` Note that you will need to use a `moduleResolution` of `nodenext` (and not `node`) in your TypeScript project in order to take advantage of this. [1]: nodejs/node#30944 [2]: https://www.npmjs.com/package/json5 --- Co-authored-by: Maarten Zuidhoorn <[email protected]> Co-authored-by: legobeat <[email protected]>
Fix files specified on the command line not linting as expected due to the generated patterns not working as intended if relative path that contains a file and not a directory due to an issue with CodeNarc pattern processing. Fix incompatible arguments passed CodeNarc by: * Using arrays internally to avoid issues with spaces in arguments being interpreted incorrectly. * Stripping all quotes from string arguments as CodeNarc doesn't handle them correctly. Fix command line -ext extensions not being processed correctly and matching too many files as it was missing the prefix. Ensure readFile and writeFile calls produce a stack trace on failure due to: nodejs/node#30944. Fix file delete race condition and variable clean up due to missing await. Fix use of includes instead of exclude parameters. Also: * Fixed Request failed logging. * Fix README.md typo * Add additional useful debug logging. * Run dev:pre-commit to update CHANGELOG.md * Add more cspell entries * Add missing items to CodeNarcServer.groovy usage. * Re-enable tests which are now fixed.
Fix files specified on the command line not linting as expected due to the generated patterns not working as intended if relative path that contains a file and not a directory due to an issue with CodeNarc pattern processing. Fix incompatible arguments passed CodeNarc by: * Using arrays internally to avoid issues with spaces in arguments being interpreted incorrectly. * Stripping all quotes from string arguments as CodeNarc doesn't handle them correctly. * Enable automatic argument quoting on Windows. Fix command line -ext extensions not being processed correctly and matching too many files as it was missing the prefix. Ensure readFile and writeFile calls produce a stack trace on failure due to: nodejs/node#30944. Fix file delete race condition and variable clean up due to missing await. Fix use of includes instead of exclude parameters. Also: * Fixed Request failed logging * Fix README.md typo * Add additional useful debug logging * Run dev:pre-commit to update CHANGELOG.md * Add more cspell entries * Add missing items to CodeNarcServer.groovy usage * Re-enable tests which are now fixed * Override axios for security patch
Fix files specified on the command line not linting as expected due to the generated patterns not working as intended if relative path that contains a file and not a directory due to an issue with CodeNarc pattern processing. Fix incompatible arguments passed CodeNarc by: * Using arrays internally to avoid issues with spaces in arguments being interpreted incorrectly. * Stripping all quotes from string arguments as CodeNarc doesn't handle them correctly. * Enable automatic argument quoting on Windows. Fix command line -ext extensions not being processed correctly and matching too many files as it was missing the prefix. Ensure readFile and writeFile calls produce a stack trace on failure due to: nodejs/node#30944. Fix file delete race condition and variable clean up due to missing await. Fix use of includes instead of exclude parameters. Also: * Fixed Request failed logging * Fix README.md typo * Add additional useful debug logging * Run dev:pre-commit to update CHANGELOG.md * Add more cspell entries * Add missing items to CodeNarcServer.groovy usage * Re-enable tests which are now fixed * Override axios for security patch
Fix files specified on the command line not linting as expected due to the generated patterns not working as intended if relative path that contains a file and not a directory due to an issue with CodeNarc pattern processing. Fix incompatible arguments passed to CodeNarc by: * Using arrays internally to avoid issues with spaces in arguments being interpreted incorrectly. * Stripping all quotes from string arguments as CodeNarc doesn't handle them correctly. * Enable automatic argument quoting on Windows. Fix command line -ext extensions not being processed correctly and matching too many files as it was missing the prefix. Ensure readFile and writeFile calls produce a stack trace on failure due to: nodejs/node#30944. Fix file delete race condition and variable clean up due to missing await. Fix use of includes instead of exclude parameters. Also: * Fixed Request failed logging * Fix README.md typo * Add additional useful debug logging * Run dev:pre-commit to update CHANGELOG.md * Add more cspell entries * Add missing items to CodeNarcServer.groovy usage * Re-enable tests which are now fixed * Override axios for security patch
Fix files specified on the command line not linting as expected due to the generated patterns not working as intended if relative path that contains a file and not a directory due to an issue with CodeNarc pattern processing. Fix incompatible arguments passed to CodeNarc by: * Using arrays internally to avoid issues with spaces in arguments being interpreted incorrectly. * Stripping all quotes from string arguments as CodeNarc doesn't handle them correctly. * Enable automatic argument quoting on Windows. Fix command line -ext extensions not being processed correctly and matching too many files as it was missing the prefix. Ensure readFile and writeFile calls produce a stack trace on failure due to: nodejs/node#30944. Fix file delete race condition and variable clean up due to missing await. Fix use of includes instead of exclude parameters. Also: * Fixed Request failed logging * Fix README.md typo * Add additional useful debug logging * Run dev:pre-commit to update CHANGELOG.md * Add more cspell entries * Add missing items to CodeNarcServer.groovy usage * Re-enable tests which are now fixed * Override axios for security patch
Fix files specified on the command line not linting as expected due to the generated patterns not working as intended if relative path that contains a file and not a directory due to an issue with CodeNarc pattern processing. Fix incompatible arguments passed to CodeNarc by: * Using arrays internally to avoid issues with spaces in arguments being interpreted incorrectly. * Stripping all quotes from string arguments as CodeNarc doesn't handle them correctly. * Enable automatic argument quoting on Windows. Fix command line -ext extensions not being processed correctly and matching too many files as it was missing the prefix. Ensure readFile and writeFile calls produce a stack trace on failure due to: nodejs/node#30944. Fix file delete race condition and variable clean up due to missing await. Fix use of includes instead of exclude parameters. Also: * Fixed Request failed logging * Fix README.md typo * Add additional useful debug logging * Run dev:pre-commit to update CHANGELOG.md * Add more cspell entries * Add missing items to CodeNarcServer.groovy usage * Re-enable tests which are now fixed * Override axios for security patch
What is the expected output?
The err.stack should contain error text and stack
Same as readFileSync function:
What do you see instead?
err.stack only contains error text
fs.writeFile
have same troubleАnother strange thing is that the error text is also different (local/absolute path)
The text was updated successfully, but these errors were encountered: