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

Improve error handling for web.load when a non-existent task ID is provided. #2154

Open
FilipeFcp opened this issue Jan 10, 2025 · 8 comments
Assignees

Comments

@FilipeFcp
Copy link
Contributor

A user pointed out that when an incorrect task-id is provided to the web.load function, it leads to a Python error, with a lot of output messages that are not useful for the user.

If possible, a clear message that the task-id doesn't exist could be more informative.

priority: small

@momchil-flex
Copy link
Collaborator

What do you think @QingengWei

@QingengWei
Copy link

What do you think @QingengWei

Web API returns a 404 NOT FOUND error, but the Python client changes the error to None: https://github.com/flexcompute/tidy3d/blob/develop/tidy3d/web/core/http_util.py#L132-L133

@momchil-flex
Copy link
Collaborator

That's funny, who knows where this comes from you or maybe even Lei Zheng in the first days of the web api? Could you propose a change?

@QingengWei
Copy link

QingengWei commented Jan 14, 2025

That's funny, who knows where this comes from you or maybe even Lei Zheng in the first days of the web api? Could you propose a change?
image
Do you know who they are?

I changed the error code from Web API, and now the error info in Python client is as follows:
image

@yaugenst-flex
Copy link
Collaborator

We should probably just handle status code correctly in the client though I'd say?

@QingengWei
Copy link

We should probably just handle status code correctly in the client though I'd say?

I don't know the historical reason. I handle it from the web api and the client does not make any changes.

@yaugenst-flex
Copy link
Collaborator

I understand the reasoning, but returning a different error code just to avoid triggering try/except None creates two issues:

  1. The frontend still does not handle 404 errors correctly.
  2. The web API now returns an incorrect or unexpected status code when a simulation is not found.

Instead of introducing workarounds, we should address the root cause by fixing the frontend to properly handle 404 responses. This ensures both the API and client behave as expected without compromising on correctness.

@QingengWei
Copy link

QingengWei commented Jan 14, 2025

I understand the reasoning, but returning a different error code just to avoid triggering try/except None creates two issues:

  1. The front end still does not handle 404 errors correctly.
  2. The web API now returns an incorrect or unexpected status code when a simulation is not found.

Instead of introducing workarounds, we should address the root cause by fixing the front end to properly handle 404 responses. This ensures both the API and client behave as expected without compromising on correctness.

From the Python code now, it seems not to care about the error code but the error message. Changing python client code can fix the root cause.

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

No branches or pull requests

5 participants