LicenseFinder icon indicating copy to clipboard operation
LicenseFinder copied to clipboard

Improve Error Handling

Open eode opened this issue 3 years ago • 4 comments

Right now, I think this is the situation:

  • If the error code is 0
    • if there is text on stderr starting with "ERROR", then there was an error accepting arguments.
    • otherwise, the command was run successfully and the result was that no action items were present.
  • If the error code is 1:
    • action items were present, or perhaps other errors occurred.

..this is pretty painful. How about:

  • return 0 when there are no errors
  • return a code (at least somewhat) unique to the error when there are errors

Example: 0 for everything is fine and all licenses passed 1 for everything is fine, but there are action items 2 for incorrect arguments 3 environmental error 4 ...etc, etc.

At the very least, it would be nice to distinguish between "Something is wrong with the licenses" and "Something is wrong."

eode avatar May 20 '22 00:05 eode

We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story.

The labels on this github issue will be updated when the story is started.

cf-gitbot avatar May 20 '22 00:05 cf-gitbot

Hey @eode ! So you want the if there is text on stderr starting with "ERROR", then there was an error accepting arguments. case moved out of exit 0 and you want action items split out from other errors?

This seems fine to me but there may be some backwards compatibility issues as it is changing behaviour. If we leave action items present as exit 1 and no license issues as exit 0, it should be fine.

If you want to make a PR for this, I would be happy to take a look!

xtreme-shane-lattanzio avatar Jun 27 '22 16:06 xtreme-shane-lattanzio

Basically, yes. Typically, Exit 0 is for "exiting without any errors", and other exit codes indicate some kind of error.

Of course, exit codes can be made more specific -- 1 for action items, 2 for bad command line, 3 for missing dependencies, etc. ..but for typical CI, you're only checking for pass/fail, and simply moving any errors out of exit 0 is sufficient.

My main reason for this is:

  • if a change in the environment causes an incorrect command line -- say, a folder structure is changed, then the result won't error out, and instead CI continues to pass

IMO, only a successful evaluation of the licenses with no action items should return 0. Everything else is something that needs operator attention, and should return nonzero.

A good minimal set of error codes would be:

  • 0 - Licenses were evaluated and no action items were found
  • 1 - Licenses were evaluated and action items were found
  • 2 - Licenses were not evaluated
    • Returned any time no license comparison is done, when one is expected.
      • A project root was specified, but not found
      • Bad command line
      • Bad configuration file
      • Parameters can't be fulfilled completely

Of course, 2 could be broken down into 2, 3, 4 etc., but the minimal case covers necessity quite well.

eode avatar Jul 01 '22 15:07 eode

re: PR: Unfortunately, I don't know Ruby, and I don't have time to learn the language. I can provide assistance in other ways if needed, though.

eode avatar Jul 01 '22 15:07 eode