Make error handling in `api.py` less verbose/repetitive

[jotaen4tinypilot]

In https://github.com/tiny-pilot/tinypilot-pro/pull/364#pullrequestreview-862312155 the idea came up to centralize the error handling of the API layer in the backend. Currently, we need to write the same try/except all over again, but the only reason we do this is for assigning HTTP status codes.

Example: In the Pro api.py, we catch 17× request_parsers.errors and then return status 400. Then there are an additional 34× where we have an except block that maps to a specific error code.

I’d spontaneously see multiple ways how we could do this: (there might be others/more)

• (a) We could have a central mapping between error type and HTTP code (as suggested by @jdeanwallace in the original comment)
  • Pro: maximum code clean-up in api.py, since we wouldn’t need any try/except at all.
  • Con: harder to see what status codes an endpoint can return (on the endpoint directly)
• (b) Maybe it’s possible to have a “per-endpoint” decorator, that let’s us define a mapping. Like:

  @api_blueprint.route('/massStorage/backingFiles/<backing_file_name>/fetchFromUrl', methods=['PUT'])
  @errormapping({
    mass_storage.DiskSpaceExceededError: 507,
    mass_storage.FilenameAlreadyExistsError: 409,
  })
  def mass_storage_backing_files_fetch_from_url(backing_file_name):
  # ...
  

  We could probably also do that less fancy as function call in the method body directly.

  @api_blueprint.route('/massStorage/backingFiles/<backing_file_name>/fetchFromUrl', methods=['PUT'])
  def mass_storage_backing_files_fetch_from_url(backing_file_name):
  try:
    # something
  except mass_storage.Error as e:
    return map_error({
      mass_storage.DiskSpaceExceededError: 507,
      mass_storage.FilenameAlreadyExistsError: 409,
    }, e)
  

  • Pro: declarative and without repetition
  • Con: I don’t think that exists, so we’d need to write it ourselves
• (c) We could already define the HTTP code in the error class itself. (E.g. here)
  • Pro: easy to maintain and to see
  • Con: the domain modules actually don’t know anything about HTTP, so we’d weaken our abstraction boundaries.
• (d) We introduce central mapping for certain generic errors, that are common and “trivial” to handle. (I think that’s probably only request_parsers.errors.Error, and I’ve demonstrated it in this quick POC).
  • Pro: avoids “trivial” repetition in the endpoints
  • Con: we’d have error handling in two places (even though for 500 we do have that already).

Not sure how much effort this is worth investing in. On the other hand, out current approach is indeed quite verbose/repetitive for no real “benefit”, and api.py continues to grow.

[jotaen4tinypilot]

It’s also worth keeping in mind that we don’t rely on HTTP status codes that much anyway, but we assign string codes in case we need to distinguish errors in the frontend.

So a refactoring like this would be mostly for code sanity, not so much for improved functionality.