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

Handle exceptions as annotated more #143

Merged
merged 4 commits into from
Jan 31, 2024
Merged

Handle exceptions as annotated more #143

merged 4 commits into from
Jan 31, 2024

Conversation

pbrisbin
Copy link
Member

@pbrisbin pbrisbin commented Jan 26, 2024

Catch query-cancelled as an AnnotatedException

412b74b

Because of how our catch functions work, this will be behavior neutral
if the exception is not annotated (with a minor improvement in the
structure of the logged error message), but if/once it is annotated,
we'll include an error.stack value.

Catch exceptions as AnnotatedException in Kafka module

d066e12

Because of how our catch functions work, this will be behavior neutral
for exceptions that not annotated (with a minor improvement in the
structure of the logged error message), but if they are annotated,
we'll now include an error.stack value.

Add withException to Frontrow.App.Exception

187896d

Add logExceptionsMiddleware

9e5404c

We currently log all handler exceptions through the errorHandler
member of the Yesod type-class. This is not ideal because it only has
access to an InternalError Text value, in which any exception has
already been turned into its string representation. This means we get an
ugly message like "AnnotatedException {...}" and aren't able to
actually construct a structured log.

We should stop doing that, and recover that logging by adding a Yesod
middleware that catches, logs, and re-throws all exceptions instead.

Then it can be handled as an actual (annotated) exception, and produce a
nice, structured log message (i.e. with error.stack). That's what this
middleware does. It's also what necessitated adding withException.

Because of how our catch functions work, this will be behavior neutral
if the exception is not annotated (with a minor improvement in the
structure of the logged error message), but if/once it *is* annotated,
we'll include an `error.stack` value.
Because of how our catch functions work, this will be behavior neutral
for exceptions that not annotated (with a minor improvement in the
structure of the logged error message), but if they *are* annotated,
we'll now include an `error.stack` value.
We currently log all handler exceptions through the `errorHandler`
member of the `Yesod` type-class. This is not ideal because it only has
access to an `InternalError Text` value, in which any exception has
already been turned into its string representation. This means we get an
ugly message like `"AnnotatedException {...}"` and aren't able to
actually construct a structured log.

We should stop doing that, and recover that logging by adding a Yesod
middleware that catches, logs, and re-throws all exceptions instead.

Then it can be handled as an actual (annotated) exception, and produce a
nice, structured log message (i.e. with `error.stack`). That's what this
middleware does. It's also what necessitated adding `withException`.
@pbrisbin pbrisbin changed the title pb/exceptions Handle exceptions as annotated more Jan 26, 2024
@pbrisbin pbrisbin requested a review from chris-martin January 26, 2024 14:48
@pbrisbin pbrisbin marked this pull request as ready for review January 26, 2024 14:48
Copy link
Contributor

@chris-martin chris-martin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

@pbrisbin pbrisbin merged commit 3144114 into main Jan 31, 2024
7 of 8 checks passed
@pbrisbin pbrisbin deleted the pb/exceptions branch January 31, 2024 14:52
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

Successfully merging this pull request may close these issues.

2 participants