Enhance error handling and refactor API request logic #27
Enhance error handling and refactor API request logic #27MontrealSergiy merged 7 commits intoaces:mainfrom
Conversation
|
In the first line looks like there the message is caused by is a bug in CBRAIN, IMHO you should make clear that error message is from CBRAIN and not your client. I submitted a PR for that CBRAIN bug a few days ago, and it should be reviewed soon, but something like that potentially can happen again when new bug is introduced or discovered. Maybe add a prefix 'Server error:', 'API Endpoint encountered error:' @prioux @dlq @natacha-beck ? In the second line the message seems tractable, IMHO |
MontrealSergiy
left a comment
There was a problem hiding this comment.
Based on your example, I think even if we extract the detailed server message, we should still communicate the status code. More, for less savvy users, we can also provide a short description of what it means, something akin
Server error (5xx): ...
Validation error (422): ...
|
Oh, do you want to add status code like - |
|
Not sure I think Record not Found is 404 ( a client/user side error). All the 5xx (500 - 505) codes are some server side errors. Can we keep this distinction? |
There was a problem hiding this comment.
I preferred the older, more user-tailored error messages, but this way - returning only server errors - might be easier to maintain and works well for advanced users.
There are a few cases where a server error message isn’t returned, but that doesn’t bother me much.
I do feel that sometimes the wording could be slightly improved. For example, instead of:
Try with authorized access
it would be clearer to show:
Error: Access denied. Please log in using authorized credentials.


Tokens work as long as the server says they work.
Enhance error handling and refactor API request logic across multiple modules.
Improved HTTP error responses in
cli_utils.py, streamlined data fetching indatamodules, and removed redundant error handling.Updated command argument parsing in
main.pyto ensure required fields are enforced.Cleaned up interactive prompts in
tags.pyand adjusted formatting in output functions.Fixes: #5