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

Do not fail miserably if we don't have proper json during error 500 #220

Merged
merged 1 commit into from Dec 18, 2018

Conversation

Projects
None yet
2 participants
@alexAubin
Copy link
Member

alexAubin commented Dec 14, 2018

Problem

Currently, if we encounter an error 500, we assume we got a json with a few properties defined and attempt to parse it. However for some reason we might not get such a json and the admin will fail miserably and pacman will keep running forever (and nothing is displayed to the user).

This is related to YunoHost/issues#1196 though I don't expect if to be 100% of the root cause.

Solution

I don't know what's a proper solution, I just added a quick and dirty try/catch so that the code doesn't fail miserably and shows the raw xhr.responseText as traceback. At least pacman stops moving and something is shown to the user :/

@alexAubin

This comment has been minimized.

Copy link
Member Author

alexAubin commented Dec 14, 2018

Digging a bit more this, I have found that this issue might be way easier to encounter that I thought. The piece of javascript was assuming that the only piece of code who might generated 500 errors was this one :

https://github.com/YunoHost/moulinette/blob/84c9a74d3380f59cdc9fda6aa5bf5fac9d619a0c/moulinette/interfaces/api.py#L446

In which we fed proper json-formatted data. However, I now realize that here :

https://github.com/YunoHost/moulinette/blob/84c9a74d3380f59cdc9fda6aa5bf5fac9d619a0c/moulinette/interfaces/api.py#L544-L550

some "simple" MoulinetteError with specific errno where trigerring 500 errors with just a raw string. Which will then be triggering the issue adressed by this PR. So that explains at least a lot of "infinite pacman" symptoms which were reported...

@zamentur
Copy link
Contributor

zamentur left a comment

LGTM

@alexAubin alexAubin added this to the 3.4.x milestone Dec 18, 2018

@alexAubin alexAubin merged commit a586e8d into stretch-unstable Dec 18, 2018

@alexAubin alexAubin deleted the do-not-fail-miserably-on-error-500-bad-json branch Dec 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment