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

[benoit74]

What I observed is that when a recipe is created on Zimfarm and it fails, zimit-frontend API is always returning a 500 error and no error message is provided to the UI.

Looking at the code, I realize there are 3 operations performed on the Zimfarm when someone requests a youzim.it execution: the schedule creation, the requested task creation and the schedule deletion.

For every of these requests, there are many different situations to take into account when speaking about a failing HTTP request:

1. Zimfarm returns a 400 error with error description, i.e. an explanation about which recipe flag(s) is(are) incorrect
2. Zimfarm returns a 400 error without error description
3. Zimfarm returns a 4xx error
4. Zimfarm returns a 5xx error
5. Zimfarm call fails at transport level (timeout, DNS issue, ...)
6. Zimfarm returns a success code but we do not find what we expect in the response

Currently the code:

• raises an InternalError exception when schedule creation fails (no matter the reason)
• does not check the response of schedule creation (not very useful indeed)
• raises an InternalError exception when requested task creation fails
• raises a ValueError exception when we do not find the expected task ID in the response of requested task creation
• only logs details when the schedule deletion fails and continue with normal operation

ValueError exceptions are not handled by web server so they probably lead to 500 error at HTTP level (if I'm not mistaken).

InternalError exceptions are handled as 500 error but the exception message is lost.

A minimal intervention would be to return the InternalError message in the HTTP response and display it in the UI, however I'm not convinced that it always make sense for all situations. Or I'm even convinced it does not makes sense in many situation. Many error could be pretty obscure to the end user (and this is most probably why it has been done like that so far).

What I would suggest is:

1. when the Zimfarm returns a 400 error with error description, forward the error description in the API response and display this description in the UI
2. when there is a transport level error, return a 503 error and process 503 error at the UI level with a nice message indicating that the service is currently unavailable but the user should try again in few hours
3. replace ValueError by InternalError, since this does not make a difference (except if I'm mistaken)
4. return InternalError message to the UI but do not display it (developers / tech users could more easily identify the issue but it is too complex information to present to the regular user)

[benoit74]

@kelson42 @rgaudin do you have any PoV on this suggestion, do you need to discuss it live?

Should we fix this before Zimit 2.0? I think so because these errors will become more common once we check validity of recipe flags in zimfarm, and without a change like this one the end-user will have no clue about whether something is wrong in his setting - and what - or if something else is not working correctly

[rgaudin]

these errors will become more common once we check validity of recipe flags in zimfarm

In theory, yes, given we check something that wasn't before, chances of failures are higher but in practice it only affects zimit-frontend API not respecting zimit-farm-API expected flags so this would be rare… and this is not something user can be much about.

User-input related checks should be in UI and to some extend in zimit-frontend.

Not saying we shouldn't do it, just wanted to clarify the actual impact. Less important than having the UI properly set maxLength on fields for instance.

[benoit74]

I do not expect this to be rare given the number of error for "Description is to long" we got recently ^^

But I agree that #41 is important as well (but needs implementation of upstream change in zimfarm)

[rgaudin]

I do not expect this to be rare given the number of error for "Description is to long" we got recently ^^

That's zimit-frontend's job to not allow such things

[benoit74]

That's zimit-frontend's job to not allow such things

But zimit-frontend will only do it once the zimfarm API informs that the description field has a maximum length of 80 chars, or do I miss something? (or we could do it manually for every fields in zimit-frontend, but I don't think it would be a good idea for long term maintenance)

[rgaudin]

UI should not allow setting incorrect values. That's why this piece of code is shared with ZF directly (I believe). It's a ZF ticket AFAIK.

Additional checks could (don't think there is) be performed on zimit-frontend but prompting youzim.it users with the text message accompanying 400 responses from an external API is not a good practice. We want to abstract all this.

Sure it may happen (unexpected change) but that's graceful bug handling, not normal behavior. Hence my opinion: nice to have but clearly less important than ZF widgets preventing incorrect values

[benoit74]

As discussed today in Kiwix weekly with @kelson42, we:

• must move on this issue ASAP to provide useful hints to the end-user about which error he made in his recipe flags ; this is a short-term move
• plan the long-term nicer solution which is based on Validate configuration for minLength/maxLength and pattern #41 (short term solution will nevertheless still be needed / useful)