Project

General

Profile

Actions

Bug #3416

closed

[API] Clean up exception raise/rescue usage, use 4xx and 5xx appropriately

Added by Tom Clegg over 9 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Assigned To:
-
Category:
-
Target version:
-
Story points:
1.0

Description

Background:
  • Most exceptions in API server result in 422, even if they arise from server bugs that the client doesn't have any hope of fixing.
  • There is a specific list of exceptions that are rendered as 404.
  • The render_error action asks the exception object for an http_status (if respond_to?(:http_status)). PermissionDeniedError uses this.
Desired behavior:
  • Model validation errors, and similar exceptions we generate in normal server operation, should render 422.
  • Exceptions other than the expected ones should render 500.
Implementation notes:
  • In render_error, default to 500.
  • Make a subclass of StandardError (something like RequestError) with an http_status method that returns 422.
  • Subclass RequestError with more specific error classes, returning different http_status if appropriate. For example, ArvadosModel::PermissionDeniedError should subclass RequestError instead of StandardError.
  • Use these new exceptions in some of the more obvious places. Over time we'll catch more 500s that should be 400s, and fix them.
Perhaps tangential, but in the same code:
  • Report the name of the exception class in a separate hash key ("error_class"?) if the HTTP status code is 4xx. This can make it possible for clients to react appropriately to specific errors without having to parse the human-readable error message.
  • In production mode, dump stack traces for 5xx but not 4xx. In dev mode, dump always. This should be accomplished by always dumping but using the appropriate log level (4xx debug vs. 5xx error) so the administrator still has the option of turning on 4xx stack traces.
Actions #1

Updated by Tom Clegg over 9 years ago

  • Description updated (diff)
Actions #2

Updated by Tom Clegg over 9 years ago

  • Target version set to Arvados Future Sprints
Actions #3

Updated by Tom Clegg over 9 years ago

  • Description updated (diff)
Actions #4

Updated by Brett Smith over 9 years ago

Please also change the Python SDK so it no longer retries 422 after this is done.

Actions #5

Updated by Peter Amstutz almost 4 years ago

  • Target version deleted (Arvados Future Sprints)
  • Status changed from New to Closed
Actions

Also available in: Atom PDF