Errors in bumpfile scripts don't change return code from 0

[mtfurlan]

If you take the example standard-version-updater.js from the readme and point to an invalid JSON file, standard-version still thinks it completed successfully and doesn't fail.
Same thing if you replace the contents of the two functions to just just throw new Error('this doesn't do anything!');

[jbottigliero]

Thanks for the report! So we have a test covering this exact case currently, but I think the fact that alterations to your local state/project still occur after the error is encountered might be the larger issue here...

Going to link this to #528 and close, but feel free to reopen/comment if I've misinterpreted your issue!

[mtfurlan]

That test case doesn't match with my experiences, I'm putting failures in my update script and standard-version isn't failing.

My test environment:

• standard-version : 7.1.0
• node: v10.15.1
• debian: 10

.versionrc

{
  "bumpFiles": [
    {
      "filename": "foo",
      "updater": "version-updater.js"
    }
  ]
}

Issue A: Invalid syntax:
version-updater.js

#!/usr/bin/env node

module.exports.readVersion = function (contents) {
    This is not a valid file

$ standard-version 
Unexpected identifier
updater.updater.readVersion is not a function
✔ outputting changes to CHANGELOG.md
✔ committing CHANGELOG.md
✔ tagging release v1.0.3
ℹ Run `git push --follow-tags origin master` to publish

Issue B: Throw an error in the updater script
version-updater.js

#!/usr/bin/env node

module.exports.readVersion = function (contents) {
    console.log("going to throw an error");
    throw new Error('This is an error');
}

module.exports.writeVersion = function (contents, version) {
    console.log("going to throw an error");
    throw new Error('This is an error');
}

$ standard-version 
going to throw an error
This is an error
✔ outputting changes to CHANGELOG.md
✔ committing CHANGELOG.md
✔ tagging release v1.0.4
ℹ Run `git push --follow-tags origin master` to publish

The return code from standard-version was 0 in both cases.

[jbottigliero]

Ah, sorry @mtfurlan for grouping this incorrectly during a triage session yesterday... it seemed similar.

At the moment, we are capturing the errors and simply pushing them to a console.warn. It seems like we need to do two things:

• Be more descriptive with the messaging.

  • In your examples, the fact it's in stdout is based on us just pushing to console.warn if an error other than ENOENT is encountered during the bump – we can certainly add more context here.

• Add the functionality loosely described in Standard version should be atomic #528 to "rollback" when an error is hit.

  • After seeing how cases like this are currently being handled it seems like we might also need to allow folks to opt-out of this behavior for "backwards compatibility".

[mtfurlan]

No worries!

What I want is a return code other than 0.
More descriptive messaging might be nice, maybe just adding "This is the output from trying to run the postbump script", but I think that is way less important than the return code.

I agree that #528 would be super helpful.
I'll just track that issue there and this issue can be about the return code?