Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When giving a file that is not JSON, there's traceback output #71

Closed
doctordesh opened this issue Sep 5, 2023 · 4 comments · Fixed by #72
Closed

When giving a file that is not JSON, there's traceback output #71

doctordesh opened this issue Sep 5, 2023 · 4 comments · Fixed by #72

Comments

@doctordesh
Copy link

Hi, thanks for a good tool. I like it!

However, this happens from time to time, that I use it with a file that is not a JSON. This is caused by a typo or other human error.

> jdiff file-a.json file-b.json 
Traceback (most recent call last):
  File "/opt/conda/bin/jdiff", line 8, in <module>
   ...
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

I think that the end user should never have to see a traceback. Wrapping this call into a try-except block to just output that file-a.json is not a valid JSON file is a resonable quality-of-life improvement. I'm a developer and understand the traceback, but someone who is not as technically savvy will not necessarily understand the quite cryptic JSONDecodeError message.

All the best

corytodd pushed a commit to corytodd/jsondiff that referenced this issue Sep 5, 2023
Do not show a stacktrace on the cli tool when an invalid file is
specified. Instead, use the new Serializer class to abstract the load
process and handle the unified ValueError. Also handle FileNotFound
because that's a fairly normal user input scenario as well. Set return
code on sys.exit to indicate abnormal exit.

Fixes xlwings#71

Signed-off-by: Cory Todd <cory.todd@canonical.com>
corytodd pushed a commit to corytodd/jsondiff that referenced this issue Sep 5, 2023
Do not show a stacktrace on the cli tool when an invalid file is
specified. Instead, use the new Serializer class to abstract the load
process and handle the unified ValueError. Also handle FileNotFound
because that's a fairly normal user input scenario as well. Set return
code on sys.exit to indicate abnormal exit.

Fixes xlwings#71

Signed-off-by: Cory Todd <cory.todd@canonical.com>
@corytodd
Copy link
Collaborator

corytodd commented Sep 5, 2023

I think this is a reasonable request. How does #72 look to you?

The API surface is a bit awkward to not break but this should be sufficient.

@doctordesh
Copy link
Author

#72 looks good!

@fzumstein
Copy link
Member

Hey @corytodd thanks! Would you be up for becoming an official maintainer of jsondiff?

@corytodd
Copy link
Collaborator

corytodd commented Sep 6, 2023

Absolutely @fzumstein, happy to help :)

corytodd added a commit that referenced this issue Sep 6, 2023
For the CLI tool, typical user input should not produce stack traces. Show appropriate messages when one or both file
inputs are invalid or do not exist. This implementation respects the current API by not introducing a unified exception
type directly in the loader implementations. Current library consumers should not be broken by this change but cli users
should see a prettier output when things go wrong.

Fixes #71
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants