Fix:Error message for invalid task ID #2345
Conversation
rahul-flex
force-pushed
the
rahul-flex/webapi-errors
branch
from
b55cbb0 to
0428080
Compare
There was a problem hiding this comment.
Thanks @rahul-flex! This is going in the right direction but we should try to handle 404 errors more generally.
tidy3d/web/core/http_util.py
Outdated
| raise TaskNotFoundError( | ||
| "The requested task ID does not exist. Please verify your task ID." | ||
| ) |
There was a problem hiding this comment.
This is much more general than not finding a task. Any request that returns a 404 status code will be caught here, so this should also raise a more generic exception. Maybe something like WebNotFoundError(WebError).
The error then needs to be handled properly by the caller. We can focus on tasks for now since that was the original intention. A good place to catch this is probably in tidy3d/web/core/task_core.py
rahul-flex
force-pushed
the
rahul-flex/webapi-errors
branch
from
61f4c3b to
d46649b
Compare
|
Updated error message: |
tidy3d/web/core/exceptions.py
Outdated
| class TaskNotFoundError(WebError): | ||
| """Raised when a requested task ID does not exist on the server.""" | ||
|
|
||
| pass |
There was a problem hiding this comment.
in principle i'm not opposed to having a fine grained error like this but on the other hand it seems like just raising a WebNotFoundError should be fine. so maybe remove this one?
tidy3d/web/core/task_core.py
Outdated
| except WebNotFoundError: | ||
| raise TaskNotFoundError(f"The requested task ID '{task_id}' does not exist.") |
There was a problem hiding this comment.
i'd probably do the following instead:
except WebNotFoundError as e:
td.log.error("error message")
raise e
rahul-flex
force-pushed
the
rahul-flex/webapi-errors
branch
from
03884b5 to
6b86fa0
Compare
|
Closes #2154 |
There was a problem hiding this comment.
Thanks @rahul-flex looks good to go from my side, minus the little changelog clarification.
| @@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
| - `fill` and `fill_structures` argument in `td.Simulation.plot_structures()` and `td.Simulation.plot()` respectively to disable fill and plot outlines of structures only. | |||
| - New subpixel averaging option `ContourPathAveraging` applied to dielectric material boundaries. | |||
|
|
|||
| ### Changed | |||
| - Error message for invalid task ID. | |||
There was a problem hiding this comment.
Improved error message and handling when attempting to load a non-existent task ID.
|
@QingengWei could you have a look? This addresses the discussion in #2154. The The old |
Previously, when web.load received a 404 response for an invalid or non-existent task ID, the client would return None and produce unhelpful error messages. This commit changes that behavior: it now raises a TaskNotFoundError with a clear, user-friendly explanation.
