Handle validation exceptions. #124
Conversation
src/index.ts
Outdated
| try { | ||
| result = validator.validateFile(secondArg, options); | ||
| } catch(err) { | ||
| console.log('Unable to parse template!'); |
There was a problem hiding this comment.
I think something to spur the user on to resolve the issue would be helpful here.
"Unable to parse template! Is the template formatted correctly?"
"Unable to parse template! Does the path exist?"
"Unable to parse template! Use --verbose for more information."
Probably the last one is most useful.
Additionally, as we are now catching the file not found, it might be nice to return this as a separate error, so this parsing error is the last case "I've run into terrible difficulty" message, as opposed to something that could be encountered due to a typo.
There was a problem hiding this comment.
Sounds very nice. I believe that from a UX quality perspective the most specific messages are best for users however, they tend to become tedious to maintain therefore I'd like to lean on a combination of 3 and 2 implying that perhaps we should provide sufficiently specific messages for common scenarios such as invalid path and at the same time provide a broad catch-all type message for not really expected exceptions :) ... then again it's just a thought I think you have more experience with this project and perhaps see things more clearer than I do.
There was a problem hiding this comment.
I think
Unable to parse template! Use --verbose for more information. would work best for a general fall-through then.
Then we just need to add some extra logic to check if a path exists and return an error nicely.
RazzM13
force-pushed
the
uncaught_exceptions
branch
from
744fd28 to
1ed832d
Compare
| } | ||
| } | ||
| return errors['']; | ||
| }(err.message, { |
There was a problem hiding this comment.
I'm not sure... should it?
|
Interesting build failure... any ideas? |
|
Trully sorry for not catching this sooner but I was expecting the build to fail due to the different error handling messages, which I would have fixed after you've approved the last change, but certainly not in that way. |
|
No worries LGTM, just those tests to cleanup, ensuring the |
|
I've just updated the tests with the new error messages and I've had to diverge from the non-existing file exception to a general parsing exception (i.e. |
|
Good stuff! |
Fixed #121 and #122. Catches exceptions thrown in template validation and logs
Unable to parse template!tostdout, if--verboseflag provided then logs the exception tostderr. In both cases the exit code is1.