Skip to content
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

re-implement errname resolution #31

Closed
jamestalmage opened this issue Apr 28, 2016 · 2 comments
Closed

re-implement errname resolution #31

jamestalmage opened this issue Apr 28, 2016 · 2 comments

Comments

@jamestalmage
Copy link
Contributor

jamestalmage commented Apr 28, 2016

#27 dropped execFile in favor of using spawn directly, so we've lost some features.

One is whatever these two lines do:

  1. https://github.com/nodejs/node/blob/31600735f40e02935b936802ff42d66d49e9769b/lib/child_process.js#L8
  2. https://github.com/nodejs/node/blob/31600735f40e02935b936802ff42d66d49e9769b/lib/child_process.js#L204

Can we reliably just do process.binding('uv')? Or is that asking for problems. I know it means importing some native lib, but not much beyond that.

Maybe we look back all the way to Node 0.12 and make sure it was still imported and used the same back then?

@sindresorhus
Copy link
Owner

Can we reliably just do process.binding('uv')?

Yes, but we probably shouldn't as they're trying to deprecate it: nodejs/node#2768 I think we should rather open an issue requesting a public API for what we need.

@jamestalmage
Copy link
Contributor Author

they're trying to deprecate it ...

That is labeled as "stalled", and there are strong arguments for only preventing access to bindings which might be harmful (I doubt an errname lookup would count as harmful, but not sure what else the uv binding does).

I think we should rather open an issue requesting a public API for what we need

Yes, absolutely. In the meantime let's just write a defensive wrapper around process.binding('uv') so execa will only blow up if we can't access the binding, and they have a negative exit code. (it's only needed for negative exit codes). That allows us to stay backward compatible and provide the errname until a public API is available.

Assuming they deprecate before providing a public alternative, we might also be able to extract it from util._errnoException. It builds an error object from an exit code that has the errname lookup attached. That's obviously a private method, but we could just write more defensive wrappers around our access to it. That method is used liberally in the Node codebase, so no matter what, I think we will be able to hack a workable solution one way or the other until someone creates the "right way" to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants