Skip to content

refactor errors and logging#5

Open
dwgrantham wants to merge 1 commit intomainfrom
dwgrantham/refactor/logging-and-errors
Open

refactor errors and logging#5
dwgrantham wants to merge 1 commit intomainfrom
dwgrantham/refactor/logging-and-errors

Conversation

@dwgrantham
Copy link
Copy Markdown

This refactors errors and logging

  • added context to logging
  • unified errors so that they build error chains, have categories as well as context

Signed-off-by: Dave Grantham dgrantham@polygon.technology

Signed-off-by: Dave Grantham <dgrantham@polygon.technology>
@dwgrantham dwgrantham self-assigned this Dec 18, 2025
@dwgrantham dwgrantham requested a review from adamdossa December 18, 2025 00:56
@dwgrantham
Copy link
Copy Markdown
Author

Here's the error and logging refactor. I haven't been able to set up a proper testing environment yet or try against the staging network/datadog. Let's get that done ASAP so I can see what kind of errors and logging comes out of this. It looks good in my head but it might be a different story in production.

Copy link
Copy Markdown

@MaximusHaximus MaximusHaximus left a comment

Choose a reason for hiding this comment

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

Just had one question about consistency in returning structured errors in responses to clients -- as long as DD is handling the error output as expected this LGTM shipping-wise 👍

)
.into_response(),
// Chain errors and internal errors return structured error responses
ref err @ FacilitatorLocalError::Evm(_)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤔 hmm -- I'm curious why only this subset of errors are being transformed into structured errors -- shouldn't all errors be returned as structured responses if we're going to introduce them to the system?

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