Add OasstError exception class and corresponding exception filter

[andreaskoepf]

This PR introduces the exception class OasstError that can always be used to signal a backend error to the API client with message and open-assistant error_code. It should not be used when the message could leak security relevant information. OasstError is 'picklable'.

The ctor of OasstError takes parameters message: str, error_code: int, http_status_code: int = HTTP_400_BAD_REQUEST. The error-code is a numeric reference to the specific error-site in the backend.

An exception handler is registered which generates a json response with message and error_code (using a http-status code as specified by the OasstError class).

Resolves #55

[yk]

LGTM, left just small comments

[yk]

loguru also has logger.exception that gives a nice trace of the exception (in the current context) and annotates all parameters with their values, it's really nice to debug, but also very verbose, maybe worth thinking about

[andreaskoepf]

Yes, I had exactly that first but removed it again: For OasstErrors it is clear where they come from, e.g. they are usually post translation of generic internal errors. We could think about referencing 'inner exceptions' in the class that could be forwarded to the client when a server debug setting is True.

I was also looking into general handlers for uncaught Exceptions. IMO it would make sense to remove the general except Exception handlers and do the translation into 500 Internal Server Error centralized .. there logger.exception() would definitely make sense.

[yk]

suggestion: make these into an IntEnum, so they're still int, but typing could be used

[andreaskoepf]

Had that too initially. Also the question could be int vs. string error codes .. in general I had good experience with these site-level error codes .. it is a bit work during development but if a user sends you such a code you instantly can track where it came from (even without exposing stack-traces etc.)...

[yk]

yes I don't think any of that would change with an IntEnum. it would just be nicer to instead of having to take an int parameter, be able to take an OAsstErrorCode parameter (that behaves like an int)

[andreaskoepf]

(regarding typing .. I normally put such an exception class in a shared lib, the error_code param has type int to prevent dependencies between the base exception class and each and every error_code that would be defined in dependent libs). It would make sense to derive 'local' exception classes from it .. but I am not sure if the exception-filter type check in fast-api is polymorphic. I will try that.

[yk]

like this: https://docs.python.org/3/library/http.html#http.HTTPStatus

[andreaskoepf]

I have now used a simple IntEnum for the error codes.

Important: There was an error in api_auth(), probably introduced when DEBUG_SKIP_API_KEY_CHECK was added,
see

Open-Assistant/backend/oasst_backend/api/deps.py

Lines 55 to 57 in f208f65

	api_client = db.query(ApiClient).filter(ApiClient.api_key == api_key).first()
	if api_client is not None and api_client.enabled:
	return api_client

... if the DB query returned None the function returned None (instead of raising an Unauthorized error...)

[andreaskoepf]

To clarify: the api_auth() problem occurred when the api_key value sent with the request was not empty and both settings.DEBUG_SKIP_API_KEY_CHECK == settings.DEBUG_SKIP_API_KEY_CHECK == False and there was no matching or enabled api_client registered in the DB.