-
Notifications
You must be signed in to change notification settings - Fork 12
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
DM-43121: Set up server to client error propagation #973
Commits on Mar 8, 2024
-
Set up framework for forwarding server exceptions
Add an exception class that can be used to tag an exception as being a user-facing error message. Set up a FastAPI exception handler that can forward these error messages to the client side. Have RemoteButler raise the corresponding exception when errors are received from the server.
Configuration menu - View commit details
-
Copy full SHA for d852534 - Browse repository at this point
Copy the full SHA d852534View commit details -
Propagate MissingDatasetTypeError to client
Hoist MissingDatasetTypeError up to the top-level exceptions file, and mark it as as ButlerUserError so that it can be sent from the server to the client. This allows removing some code specific to this exception type from RemoteButler.
Configuration menu - View commit details
-
Copy full SHA for 9d2fd1d - Browse repository at this point
Copy the full SHA 9d2fd1dView commit details -
Ensure missing server error handling is not hidden
In unit tests, we now convert unhandled server errors to a special exception class 'UnhandledServerError' that is not part of Butler's usual exception hierarchy. The FastAPI TestClient by default passes unhandled exceptions up from the server to the client. This is useful behavior for unit testing because it gives you traceability from the test to the problem in the server code. However, because RemoteButler is in some ways just a proxy for the server-side Butler, we raise similar exceptions on the client and server side. Thus the default TestClient behavior can mask missing error-handling logic. The registry test suite in test_remote_butler.py does not yet have all error-handling logic in place, so it opts out of the new behavior.
Configuration menu - View commit details
-
Copy full SHA for 37955fa - Browse repository at this point
Copy the full SHA 37955faView commit details -
Remove bespoke error-handling code in RemoteButler
Remove one-off code handling FileNotFoundError and 404s in RemoteButler, replacing it with the new error propagation framework.
Configuration menu - View commit details
-
Copy full SHA for 0ca6baa - Browse repository at this point
Copy the full SHA 0ca6baaView commit details -
Rename ButlerLookupError to DatasetNotFoundError
LookupError is currently thrown from a variety of places for mostly-unrelated reasons, most of which don't need to be propagated to the server. Give it a more specific name instead.
Configuration menu - View commit details
-
Copy full SHA for f720a80 - Browse repository at this point
Copy the full SHA f720a80View commit details -
Configuration menu - View commit details
-
Copy full SHA for 5f502db - Browse repository at this point
Copy the full SHA 5f502dbView commit details -
Match RemoteButler.get() exceptions to Direct
It turns out that DirectButler typically throws two different exceptions from get(): 1. DatasetNotFoundError if the dataset can't be found in registry 2. FileNotFoundError if the file itself cannot be found or under some other error conditions (some of which involve not being able to find the dataset in registry.) Added test coverage for case #1, and fixed RemoteButler to comply with it. Loosened the tests for some unusual error conditions to allow either of the two exception types, because RemoteButler doesn't have enough information available to replicate the DirectButler behavior.
Configuration menu - View commit details
-
Copy full SHA for 4dd2f92 - Browse repository at this point
Copy the full SHA 4dd2f92View commit details
Commits on Mar 11, 2024
-
Configuration menu - View commit details
-
Copy full SHA for f6eb3c8 - Browse repository at this point
Copy the full SHA f6eb3c8View commit details