Forward error to stderr and set exitCode #158
Conversation
src/commands/eval.js
Outdated
| console.error( | ||
| util.inspect(JSON.parse(error.faunaError.requestResult.responseRaw), { | ||
| depth: null, | ||
| compact: false, | ||
| }) | ||
| ) | ||
|
|
||
| process.exit(1) |
There was a problem hiding this comment.
I would rather have something on the lines of:
| console.error( | |
| util.inspect(JSON.parse(error.faunaError.requestResult.responseRaw), { | |
| depth: null, | |
| compact: false, | |
| }) | |
| ) | |
| process.exit(1) | |
| console.error(JSON.stringify(JSON.parse(error.faunaError.requestResult.responseRaw), null, 2)) | |
| errorOut(new Error(`The query #${err.queryNumber} has failed`)) |
...but it's unclear to me why it's important to keep the consistency added in #118.
I would prefer having the ability to parse the errors than having it looking fancier.
There was a problem hiding this comment.
Hey @zvictor we're going to punt on making any calls on emitting parseable errors from the shell as we'd want to make that behavior consistent across the shell, and think carefully about making error formats part of the shell's contract.
I have pushed a tweak to this section however, that leverages errorOut and maintains the formating that was applied in #118 .
There's now a test to confirm the exit code as well.
|
@fireridlle @faunaee @parkhomenko is there anything else I can improve in this PR? |
|
We need this, because currently fauna shell does not exit with error
|
|
I mentioned before but will keep repeating it over and over again: I wish fauna had an open-source experience manager of sorts. Someone to represent the community inside the company. It really sucks to make hard working contributions for a company that doesn't value them. |
Hey @zvictor thanks for time and feedback and sorry we left you hanging for so long. We are working on building some automation to get these issues in front of our dev team to do better in the future. In the meantime, I'll be looking at this PR today as it does indeed look like something we need to do. |
|
Ah sorry working on fixing the mess I just made. |
Thanks for bringing up the same issue in the upload-graph-ql command @aprilmintacpineda. I've cut a separate issue to tract that: #180 |
|
This will be released today. |
|
Waiting for release! |
Its in 0.15.0 @aprilmintacpineda |
|
Thank you @cleve-fauna! I can confirm that it works much better now, throwing the errors as expected. If you feel that you can help me there as well, please don't feel shy 😊 |
This is a fix for #157 and improves the code added in #118.