Exception handling

[gingermusketeer]

At the moment there is an issue where some commands fail due to a missing function (will send a PR for this). The result is that the user sees:

I think this should be improved because:

• Error reports have limited information.
• Users don't have any context as to what might have caused the issue and so cannot infer if they have made any mistakes that are not handled.

The above error message is displayed due to:

cli/commands/package.js

Lines 135 to 136 in 9476ba3

	} catch (err) {
	spinner.warn(err.message);

From what I have seen the following error handling approaches are being used:

1. Log message and discard the stack trace
2. Raise new error with or without the original error message discarding the stack trace.
3. Ignore all errors.

1. Centralise terminating errors

Could have a function along the lines of handleAndExit(error, "Unexpected error occurred while publishing to eik server")

This could either always show the stack trace or only when debug mode is enabled. For example

An unexpected error occurred try again with --debug to see details

2. Wrap thrown errors with new error

Never discard the original error so that the context is always available to be shown. Could use something like https://www.npmjs.com/package/composite-error to do this.

For example:

catch (err) {
  throw new Error(
      `Unable to fetch package metadata from server: ${err.message}`,
  );
}

would become:

catch (err) {
  throw new WrappedError(
       err,
      `Unable to fetch package metadata from server: ${err.message}`,
  );
}

3. Check to see what sort of error is being thrown before ignoring it.

Instead of:

} catch (err) {
    return {};
}

could do something like:

} catch (err) {
   if(!jsonParseError(error) && !readFileError(err) throw err
    return {};
}

What do you think?

[digitalsadhu]

Yea. Was thinking something similar this morning when I was debugging the mayflies thing. One thing we could do is only show stack traces and internal errors when the user uses the --debug flag. I'm a bit back and forwards with myself as to whether its best to just throw such errors. Since they are not developer/user errors but bugs maybe it is best to just spew them out and get them fixed.

I think maybe your final suggestion might be best, don't spew out the internals when the errors are developer errors. Do spew out the errors when there is a problem with the system.

[trygve-lie]

I think maybe your final suggestion might be best, don't spew out the internals when the errors are developer errors. Do spew out the errors when there is a problem with the system.

I think this is an approach I would prefer. When ex its not possible to reach the server, its very helpful to get a clear message saying so. In this cases an stack trace have zero value. Though; if something blow up in the code, the stack trace are to a lot of help. It also might be an dev error happening once so it might not be possible to run the command with a --debug flag.