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

Logging of Exceptions #1921

Closed
alfechner opened this issue May 6, 2024 · 3 comments
Closed

Logging of Exceptions #1921

alfechner opened this issue May 6, 2024 · 3 comments

Comments

@alfechner
Copy link
Contributor

alfechner commented May 6, 2024

Let's say I want to indicate a bad request to my user.

I can either return 400 from the controller directly or I raise a BadRequestProblem that is translated to 400 by the ExceptionMiddleware.

There is a difference in logging, though: the BadRequestProblem exception will be logged as error by the ExceptionMiddleware.

In our company the SRE team is notified about errors. And we have the policy to log 4xx codes to warning instead.

In this example we have the option to not raise an exception but return 400 directly as mentioned already.

In the verification method the the JWT token we pass into our requests we cannot do that though: Raising an OAuthResponseProblem exception is logged on error. When returning None it also logs an error OAuthResponseProblem.

For my understanding the logging in ExceptionMiddleware is pretty static.

From one point of view I can understand that exceptions should be handled on application level (and the response code can be returned directly). On the other hand the framework kind of handles exceptions for us and they should not be logged I guess. For the verification method I mentioned I'd definitely need a way to not log an error on invalid tokens: Our SRE team would be alerted because somebody has an expired token.

Is there any way to customise the logging behaviour in this regard?

EDIT:

I found the add_error_handler method, that can register callbacks to handle exceptions. That works so far, except, that I need to add a error handler for each and every exception or response code. Is there a generic way to overwrite the default error handler?

Alternatively I could register one error handler for ConnexionException (base exception class). Then I could programatically decide if to handle (and not log) it. I'm not finding a way to pass it on to the default handler if I don't want to handle it, though.

@alfechner
Copy link
Contributor Author

The ExceptionMiddleware implements 3 methods handling the different kind of exceptions:

All 3 methods log the exceptions to level error. If the middleware does logging we should have a way to influence the log level I think: at least exceptions handled by the problem_handler are ... well ... handled already :)

Btw. the extended StarletteExceptionMiddleware does not log at all.

Wdyt?

@alfechner
Copy link
Contributor Author

We run into a similar issue with the validation in connexion.validators.json.

The code (1) logs an error and (2) raises an BadRequestProblem which again logs an error.

I assume that it is a pretty common thing for Ops teams to base alarms on exceptions logged.

The framework doesn't seems to consider this fact. What is your take on this? Would you be open to contribution to make Connextion more flexible in this regard?

@alfechner
Copy link
Contributor Author

Closed in favour of #1925.

@alfechner alfechner closed this as not planned Won't fix, can't repro, duplicate, stale May 13, 2024
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

1 participant