Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Fix ignoring exit code from spawnSync - run-wrangler.js #432

Merged
merged 1 commit into from
Aug 13, 2019

Conversation

defjosiah
Copy link
Contributor

@defjosiah defjosiah commented Aug 13, 2019

Fixes #433
Fixes #335

I had some api key issues while publishing a route to a specific zone.
I realized that the npm "run-wrangler" wrapper, was exiting zero when our api key was incorrect,
when I expected it to exit non-zero.

Looks like "spawnSync" returns an explicit status field, which has to be propagated up in order to
have the correct exit status.

This diff fixes that (removes an unused import, and fixes some quotes).

Reproduction:

# add wrangler to package.json (yarn add @cloudflare/wrangler -D)
[email protected] CF_API_KEY=incorrect_api_key_here wrangler publish --release
echo $? # will be zero

After this fix

[email protected] CF_API_KEY=incorrect_api_key_here wrangler publish --release
# error Command failed with exit code 1.

@defjosiah defjosiah changed the title Fix ignoring exit code from spawnSync Fix ignoring exit code from spawnSync - run-wrangler.js Aug 13, 2019
@xortive
Copy link
Contributor

xortive commented Aug 13, 2019

Good work! Sounds like this fixes #335 as well. Thank you for the PR!

Copy link
Contributor

@ashleygwilliams ashleygwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is excellent! thank you so much!

Copy link
Contributor

@EverlastingBugstopper EverlastingBugstopper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@defjosiah
Copy link
Contributor Author

Happy to help! We were publishing using serverless framework before, wrangler is so nice 😀

Copy link
Contributor

@xortive xortive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA tested the npm install, overwrites the run script successfully with the new one 👍

@EverlastingBugstopper EverlastingBugstopper merged commit 17fcc2b into cloudflare:master Aug 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@cloudflare/wrangler publish incorrectly exiting zero on error Build failure returns code 0
4 participants