fix: wrong exit code for validation error

[peter-rr]

Description

• Proposal to fix the wrong exit code when validation error occurs.

Related issue(s)

• Fixes #285

[Souvikns]

base.ts was like a base class that only implements the error handling. We kind of followed this, https://oclif.io/docs/error_handling#error-handling-in-the-catch-method, and if we throw the error oclif has a global error handler which would handle it, but it is for stuff like command not found etc. So we are doing console.error just to print the error messages.

We can have a custom error handler in the bin/run file as per https://oclif.io/docs/error_handling#binrun-catch-handler. Don't know what would be better though.

[sonarcloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[Souvikns]

  .catch((err) => {
    const oclifHandler = require('@oclif/core/handle');
    console.error(err.message);
    return oclifHandler(err.message);
  });

This code is redundant, the control is not reaching here since this.error is a logging command as per https://oclif.io/docs/commands#command-methods.

The control reaches here when we throw the error in the catch function.

this this.error function modifies the log quite a lot, This is how it looks

 ›   Error: error loading AsyncAPI document from file: ./s.yml is an invalid file path

Now if console.error and this.error is kind of same we should be able to have something like this -

export default abstract class extends Command {
  async catch(e: any) {
    console.error(`${e.name}: ${e.message}`);

    this.exit(1);
  }
}

This kind of works in terminal you run run ./bin/dev validate ./wrong.yaml, it gives

error loading AsyncAPI document from file: ./s.yml is an invalid file path

but when you run npm run test we get

10) validate
       with no arguments
         throws error message if no context file exists:
     Error: EEXIT: 1
      at Object.exit (node_modules/@oclif/core/lib/errors/index.js:22:11)
      at Validate.exit (node_modules/@oclif/core/lib/command.js:82:23)
      at Validate.catch (src/base.ts:1:1345)
      at Validate._run (node_modules/@oclif/core/lib/command.js:71:29)
      at async Config.runCommand (node_modules/@oclif/core/lib/config/config.js:226:25)
      at async Object.run (node_modules/@oclif/test/lib/command.js:21:13)
      at async Context.run (node_modules/fancy-test/lib/base.js:68:25)

---

I also found this, https://github.com/oclif/oclif/issues/229 There is a open issue in oclif and there is no fix for it yet.

[peter-rr]

Hey @Souvikns, I wonder if we could opt again for the first solution you proposed. I mean, the following one for the base.ts file:

export default abstract class extends Command {
  async catch(e: any) {
    console.error(`${e.name}: ${e.message}`);
    process.exitCode = 1;
    }
  }

The output we get would be as follows:

ValidationError: Document can't be null or falsey.
1 

And there are only 4 failing tests when this solution is applied, all of them related to missing context, which I guess it might be easier to solve:

What do you think about this? Still a possible solution? 🤔

[peter-rr]

@Souvikns Also I'd like to solve the conflicts in this branch, related to package-lock.json file, so I can fetch the latest changes from master, but I don't have write access to resolve those conflicts. Do you know how should I proceed in this case? Thanks, man :)

[sonarcloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[peter-rr]

@Souvikns I think the conflicts in the branch have been already solved with my last commit: https://github.com/asyncapi/cli/pull/288/commits/ff0422b426f5049c3f1a15ecb871cf9aefd48a1c Let me know in case these changes are not enough or I must do another adjustment :)

[derberg]

@peter-rr hey, there are more conflicts to package-lock, can you have a look?

tip how I solve such issue:

1. remove package-lock from branch and paste the one from upstream
2. commit
3. npm install
4. commit

@Souvikns can you have a look if this one is ready to approve once conflicts are solved?

[Souvikns]

Yeah Everything is working, we are getting the error code as expected just need the conflicts resolved.

[sonarcloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[derberg]

@Souvikns I fixed package-lock and also increased timeout in tests, as release is still failing. Please approve

[derberg]

/rtm

[asyncapi-bot]

:tada: This PR is included in version 0.24.1 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: