Improve error handling #462
Conversation
|
I'll update the general-cli tests shortly. |
| @@ -42,15 +51,44 @@ export async function run(_argvInput, _container) { | |||
| let subMessage = chalk.reset( | |||
| "Use 'fauna <command> --help' for more information about a command.", | |||
| ); | |||
| let epilogue = ""; | |||
There was a problem hiding this comment.
thoughts on extracting this to a helper function and typing the error object? might make it easier to recall/find how to get certain error behaviors.
edit: found the types. still think this might be worth extracting just because our runner/entrypoint is now like 90% error handler.
There was a problem hiding this comment.
I'll file it as a follow-up. My thoughts as well. It would make testing easier to since we could test error handling/printing separate from the individual messages we actually expect.
| @@ -93,7 +131,7 @@ function buildYargs(argvInput) { | |||
| }) | |||
| .command("warn", false, { | |||
| handler: async () => { | |||
| process.emitWarning("this is a warning emited on the node process"); | |||
| process.emitWarning("this is a warning emitted on the node process"); | |||
There was a problem hiding this comment.
my poor spelling getting all beaten up between you and cleve 😔
| @@ -54,12 +128,15 @@ const COMMON_QUERY_OPTIONS = { | |||
| * @param {object} argv | |||
| * @param {string} argv.database - The database to use | |||
| * @param {string} argv.secret - The secret to use | |||
| * @param {boolean} argv.local - Whether to use a local Fauna container | |||
There was a problem hiding this comment.
should really have a ticket for typing argv correctly...
src/lib/fauna.mjs
Outdated
| @@ -73,7 +77,7 @@ export const runQuery = async ({ | |||
| }) => { | |||
| // Check for required arguments. | |||
| if (!query) { | |||
| throw new Error("A query is required."); | |||
| throw new CommandError("A query is required."); | |||
There was a problem hiding this comment.
is this not validation? the line's fuzzy, but it seems to me that validation errors are sort of all errors we can determine strictly from argv without touching fs, network, etc.
There was a problem hiding this comment.
+1, because hideHelp is false on ValidationError
There was a problem hiding this comment.
I can switch it. The problem is that this is library code and we should never really get this far without a query. You can't write a helpful validation message (i.e. use [fql] or --input) in this context.
There was a problem hiding this comment.
trying a few things like leaving the query arg missing or providing an empty string catch the error before this point. So this may be moot? If the other code is working query should always be truthy by this point.
There was a problem hiding this comment.
I don't know how yet, but I would very much like to print out more info for Fauna Service errors, including error code and other information like the actual abort object for aborts and contraints for constraint errors. At least at debug level. But this looks good and I think tweaks to handling specific errors can be done in a follow up.
Co-authored-by: echo-bravo-yahoo <[email protected]>
We do in query via --extra. |
| if (e.cause) { | ||
| logger.fatal(e.cause?.stack, "error"); | ||
| } |
There was a problem hiding this comment.
don't need null coalesce here
| if (e.cause) { | |
| logger.fatal(e.cause?.stack, "error"); | |
| } | |
| if (e.cause) { | |
| logger.fatal(e.cause.stack, "error"); | |
| } |
or can cause.stack be undefined?
| if (e.cause) { | |
| logger.fatal(e.cause?.stack, "error"); | |
| } | |
| if (e.cause?.stack) { | |
| logger.fatal(e.cause.stack, "error"); | |
| } |
|
Problem
All errors are thrown and treated the same. This means validation errors, known error states, and unexpected errors get the same treatment.
Solution
This PR updates our error handling for adding capabilities for three classes of errors:
Additionally, for debugging purposes, if
causeis available on an error that will be outputted as well.Result
Validation error/unknown argument (ignore the red background, that's my shell):
Command error/not logged in
Command error/bad query syntax
Unknown error/throw
With verbosity
Testing
The tests have been updated to reflect this new behavior.