User's JSON parse error causes ugly Python exception #809#1468
User's JSON parse error causes ugly Python exception #809#1468simobasso merged 3 commits intocookiecutter:masterfrom
Conversation
|
Code Climate has analyzed commit 61175d3 and detected 0 issues on this pull request. View more on Code Climate. |
|
@noone234 thank you for contribution. At least now I understand what is #809 about. You should not remove current Raise error, as cc can be used in internal scripts and this error can be handled in some kind. Correct way to implement this: If as part of other product - only raise, as it was. @noone234 do you interested to continue work on this? |
|
@insspb Thank you for reviewing my PR. Yes, I am interested in continuing the work. I will try to do as you recommended. |
61175d3 to
c3b54c4
Compare
Steps to reproduce the issue: Refer to cookiecutter#809 1. Create a subtly malformed .json file in a project directory 2. Run cookiecutter on that project 3. Observe your eyes glazing over as a huge python exception comes out of it. Cause: In generate.py's generate_context() function, for a JSON decoding error, it raised a friendlier exception. That is still daunting to new users. Solution: cli.py's main() function already catches several exceptions. I added ContextDecodingException to that list. That makes cookiecutter print the friendlier error message and exit with a non-zero status.
c3b54c4 to
ead70e5
Compare
|
After studying the source code further, I opted for a simpler solution. cli.py's main() function already catches several exceptions. I added That has the desired effect.
|
|
@noone234 |
For this purpose, I:
* Created "tests/fake-repo-bad-json" directory.
In that directory,
* "cookiecutter.json" is a copy of
"tests/test-generate-context/invalid-syntax.json".
* The rest is a copy of "tests/fake-repo-pre".
That sufficed for this test.
f8cdfee to
c670df3
Compare
|
Added a test in test_cli.py. It is adapted from tests_generate_context.py's test_generate_context_with_json_decoding_error(). I am not sure if this suffices. |
|
Hi @insspb, I added a test in test_cli.py. Do you need anything else from me? |
|
Hi @noone234 I am currently busy on other project. Will review after weekend. |
|
@insspb bumping this PR. It has been a few weeks since last activity. |
|
@insspb Can you please remove the Do you think you will still have time to review this PR? If not, what are my next steps? Thank you. |
|
@JonZeolla Sorry to interrupt. Can you please help me get this PR unstuck or direct me to someone who can? There has been no activity since October; 4 months. |
|
@simobasso Hi, I'm reaching out for assistance. Can you please help me get this PR unstuck or direct me to someone who can? There has been no activity since October; 5 months. Per the December 28 comment, can the |
|
Hey @noone234, 👋 thank you for reaching out! Sadly I don't have any permission to help you 😞 as you can see on #1485 I'm in the same conditions. One of the maintainers said #1485 (comment) that no one has time for this project now. What can I do for you is to review your pr but I cannot merge it sorry. |
|
Hey @noone234 👋, just merged! Thanks for your job and sorry 😅 about the delay, we will do better next time! |
|
Thank you so much @simobasso! I see that you are now a maintainer. Congratulations! 🎉 |
Addresses issue #809. A malformed JSON file no longer raises an unhandled exception.
Steps to reproduce the issue:
Cause:
In generate.py's generate_context() function, for a JSON decoding error, it raised a friendlier exception. That is still daunting to new users.
Solution:
Logs an error message and exits.
Other things to note:
The message text has not changed.