-
Notifications
You must be signed in to change notification settings - Fork 11
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
exceptions for load_collection #352
Comments
With these it feels like they are better suited here: But on the other hand, it really is a bit unclear which exceptions to add to the general-purpose list and which to add to the processes. The question is how extensive the list of errors in the processes should be. The three examples sound like very specific and rare exceptions that are not really things a user could circumvent. And I'd think for the specific exceptions for processes I'd only list those that actually come from a user "mis-behavior" (like HTTP 400 errors), not from server errors (like HTTP 500 errors). So maybe to have a general rule:
All three examples above for me are server errors (although one could debate that RequestTooLarge could be a user error, but it's likely hard for users to guess what "too large" is). So I'm proposing to move this issue to openeo-api. |
I'm not sure these errors should be considered "general-purpose", as they are quite backend-dependent and specifically tied to data loading. By putting them in the process spec, a backend can easily drop or add errors, while errors.json in API should probably be a bit more standardized and rigid.
I'm not sure. It doesn't seem useful to me to leverage that distinction when deciding where to define an error. There are already plenty of errors in errors.json that are about "user mis-behavior" and not about server errors. And sometimes it's not possible to determine (in advance) whether the user or the server is "wrong" (e.g. if a file path can not be found, is it because the user made a path typo, or is it because of a wrong server-side mount?) |
If they are very specific, they should likely not be in the backend independent processes? Also, I think as a rule of thumb errors should usually be referenced in a description so that it's clear when they should be thrown.
Back-ends can still add exceptions at any point to their process definition.
Yes, but we are speaking processes here, not endpoints. errors.json has 400 errors here because it's for endpoints. You can only define them here. For processes, I'd still think my proposal above makes sense.
I don't get how this supports your other arguments. Why is this relevant here? My point still is that we can't anticipate all possible server errors and I'm not even sure we need to inform the user specifically. He can't do anything about server errors except contacting the support and wait. So it doesn't help him to know the exact reason, only the support must know it. [What I would do as a provider, is just reporting to the user "Server side error, details/a snapshot have been saved. Please contact support and provide us with the error id." Support can then look up the cause and fix it.] |
I was using this example to illustrate that the distinction between "user fault" (400) and "server fault" (500) is not always very clear. I know you have to put a status code on it, but that might be arbitrary (or biased: e.g. use 500 when in doubt). But in the end I would not use that as argument to put one in errors.json and the other in the process spec. To me the distinction between errors.json vs openeo-processes is just endpoint-related errors (former) vs process related errors (latter).
Unless the user has more control over the backend than we usually/currently assume: e.g. user is running own backend instance in some cloud-like solution. But anyway, my initial post was just minor suggestion, a backend can construct error message/info to own liking anyway. Issue importance is not proportional to the current length of this discussion 😸 Feel free to close as "no change required" |
I can also live with this distinction. In this case there no common ground for server-side errors in processes, but I think that is fine as they are often pretty specific. I've taken the chance to clarify in the API a little bit how process exceptions are meant to be defined, see the commit. Thanks. |
It might be intentional, but
load_collection
does not list possible exceptions.What can go wrong is quite backend dependent, but predefining some well chosen generic errors might improve troubleshooting for end user or backend maintainer. For example:
The text was updated successfully, but these errors were encountered: