Skip to content

refactor errors and logging - #5

Open
dwgrantham wants to merge 7 commits into
mainfrom
dwgrantham/refactor/logging-and-errors
Open

refactor errors and logging#5
dwgrantham wants to merge 7 commits into
mainfrom
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

@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.

@MaximusHaximus MaximusHaximus left a comment

Copy link
Copy Markdown

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 👍

Comment thread src/handlers.rs
)
.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.

4 participants