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

Error messages for spawned processes don't include exit status and/or stdout #291

Closed
alexcrichton opened this issue Sep 7, 2018 · 2 comments · Fixed by #387
Closed

Error messages for spawned processes don't include exit status and/or stdout #291

alexcrichton opened this issue Sep 7, 2018 · 2 comments · Fixed by #387
Labels
bug Something isn't working PR attached there's a PR open for this issue to-do stuff that needs to happen, so plz do it k thx
Milestone

Comments

@alexcrichton
Copy link
Contributor

🐛 Bug description

One issue we've run into with Cargo a lot is that processes inevitably fail for really weird reasons under really weird conditions. Currently it looks like wasm-pack will ferry along the stderr of a spawned process if the exit status doesn't indicate success, but this can often be confusing in situations such as:

  • If the binary segfaulted (for whatever reason) often no output is produced, so it'll look like a failure with no output.
  • Sometimes commands are buggy and send error information to stdout instead of stderr, also looking like a failure with no output.

Cargo's got some helpers for conveniently executing processes and wrapping it all up in an error if the process isn't successful while forwarding along all the relevant information necessary to display about the failed process.

I think that a good UI can also be preserved by perhaps doing something like whitelisting exit statuses to "if it exits with this then we don't need to display this as we know stderr contains useful information" (or something like that)

@ashleygwilliams ashleygwilliams added bug Something isn't working help wanted Extra attention is needed to-do stuff that needs to happen, so plz do it k thx labels Sep 24, 2018
@ashleygwilliams ashleygwilliams added this to the 0.6.0 milestone Sep 24, 2018
konstin added a commit to konstin/wasm-pack that referenced this issue Oct 1, 2018
@ashleygwilliams ashleygwilliams added PR attached there's a PR open for this issue and removed help wanted Extra attention is needed labels Oct 1, 2018
fitzgen added a commit that referenced this issue Oct 3, 2018
Add exit_status to cli error to fix #291
@fitzgen fitzgen reopened this Oct 3, 2018
@fitzgen
Copy link
Member

fitzgen commented Oct 3, 2018

Although #387 fixed one instance of this, I think we want to leave this open for a blanket solution for all the child processes that we spawn.

@fitzgen
Copy link
Member

fitzgen commented Oct 9, 2018

Done now that #392 landed.

@fitzgen fitzgen closed this as completed Oct 9, 2018
@ashleygwilliams ashleygwilliams modified the milestones: 0.6.0, 0.5.1 Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working PR attached there's a PR open for this issue to-do stuff that needs to happen, so plz do it k thx
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants