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

Display more useful errors in the UI #44

Merged
merged 4 commits into from
Nov 23, 2023
Merged

Display more useful errors in the UI #44

merged 4 commits into from
Nov 23, 2023

Conversation

benoit74
Copy link
Collaborator

@benoit74 benoit74 commented Nov 23, 2023

Rationale

Changes

  • add a development stack based on Docker-Compose to test the whole stuff (Zimit + Zimfarm) locally
  • simplify the Exception handling logic in Zimit API to produce consistent results / simplify code maintenance
  • in case of Zimfarm error, log the Zimfarm response in API under all all log levels (not just DEBUG) since this is useful (including for detecting what is most common user error) and not supposed to happen a lot (UI should prevent user to do silly things)
  • change first part of Zimit UI error message from "Unable to create schedule" to "Unable to request ZIM creation". At this stage of the message this is the only thing we know. Details are added on the next line (did we encountered an error at schedule creation or task request).
  • simplify second part of Zimit UI error message which was handling many potential situation:
    • stop considering that API response may contain either error or error_description or message fields ; in fact this response comes from Zimit API and we know there is only a potential error field
    • if error field is present in the API response, display it "as-is" and without Zimit API HTTP response code (this is too technical for the end user + we return almost always the same one) ; display it no matter the HTTP response code (it was only done for HTTP 400 before)
    • otherwise, display API HTTP response status code + status text message (as before)

NOTA:

  • this still displays the Zimfarm HTTP error code because it is impossible to retrieve this info from the UI otherwise, even for an expert user
  • the fact that "INTERNAL SERVER ERROR" is in capital letters is probably only a side-effect link to the fact that in the dev stack I do not use nginx reverse proxy but directly the Flask app

Screenshots

Before:
image

After, when API provides error data:
image

After, when API does not provide error data but just HTTP status code (and maybe other data we do not know how to use):
image

@benoit74 benoit74 force-pushed the enhance_errors branch 5 times, most recently from 585b096 to 40664f1 Compare November 23, 2023 13:50
@benoit74 benoit74 marked this pull request as ready for review November 23, 2023 14:02
@benoit74 benoit74 marked this pull request as draft November 23, 2023 14:05
@benoit74 benoit74 marked this pull request as ready for review November 23, 2023 14:16
@benoit74
Copy link
Collaborator Author

NOTA: CI has gone wild, most probably due to my force push, code-formating does not exists anymore and has been replaced by QA/check-qa

@benoit74 benoit74 self-assigned this Nov 23, 2023
Copy link
Member

@rgaudin rgaudin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!
Can you align the lines in workflow and merge?

.github/workflows/Publish.yml Outdated Show resolved Hide resolved
ui/src/constants.js Show resolved Hide resolved
@benoit74 benoit74 merged commit 23ded3e into main Nov 23, 2023
2 checks passed
@benoit74 benoit74 deleted the enhance_errors branch November 23, 2023 15:24
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 this pull request may close these issues.

API/UI are not providing useful hints when recipe creation fails
2 participants