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

bolt publish exit 0 despite npm error #179

Closed
bradleyayers opened this issue Jul 11, 2018 · 6 comments
Closed

bolt publish exit 0 despite npm error #179

bradleyayers opened this issue Jul 11, 2018 · 6 comments

Comments

@bradleyayers
Copy link

Title Description
Version v0.20.7
Type Issue
node v10.3.0
Operating System Linux
Short Description bolt publish executes npm publish, but when npm returns non-zero exit code it does not cause bolt to exit non-zero. Examples where npm publish can fail are missing credentials, missing --access, incompatible version of node.
@lukebatchelor
Copy link
Member

Orly? I'll try to take a look today 👌

@lukebatchelor
Copy link
Member

Ah okay. This is interesting, just thinking about the best way to resolve it.

Bolt doesn't exit automatically because we handle the errors ourselves in an external script (publish will actually return a list of packages that were successfully published and a list of unsuccessful attempts). That behaviour is probably not super consistent (commands should really use logging and exit codes, functions should return data).

We could either move the publish logic into an exposed function (similar to runWorkspaceTasks, getDependencyGraph, etc) and have the publish command just proxy to that, or we could add a flag to publish to tell it to exit on failure (either opt in or opt out).

Option 2 is probably easier, but option 1 is more correct...

@lukebatchelor
Copy link
Member

lukebatchelor commented Jul 18, 2018

I think we'll go with option 1.

@bradleyayers how much of a blocker is this for you? You could work around it pretty easily in the meantime (take a look at our script for reference). If it's urgent though, I'm happy to look at it this week?

@bradleyayers
Copy link
Author

Not a blocker for me, as it’s only happens when the NPM command fails, and that shouldn’t happen. It’s just a gotcha that I felt was worth raising so it can be fixed upstream. Thanks for looking into it!

@lukebatchelor
Copy link
Member

image

This is fixed in bolt 0.21.1!!

Thanks to @lachlanhunt for the PR.

@lukebatchelor
Copy link
Member

lukebatchelor commented Sep 6, 2018

Just an FYI, if you DO update to 0.21.1, be aware there was a minor breaking change, only if you were calling bolt.publish from a node script. You'll just need to change it to bolt.publishPackages and you'll be good though.
See #190

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