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

Log with fields on error in router #409

Open
ivaaaan opened this issue Nov 8, 2023 · 3 comments
Open

Log with fields on error in router #409

ivaaaan opened this issue Nov 8, 2023 · 3 comments

Comments

@ivaaaan
Copy link
Contributor

ivaaaan commented Nov 8, 2023

Hey. I have noticed that router does not pass fields of the message here:

if err != nil {
	h.logger.Error("Handler returned error", err, nil)
	msg.Nack()
	return
}

What do you think about adding msgFields to the Error function there?

@ivaaaan
Copy link
Contributor Author

ivaaaan commented Nov 8, 2023

Also, I think it would be nice to add correlation ID to the logs by default. I know it's something that middleware can handle, but having it in logs by default would be really nice. Or redo logger interface, so it has access to the message metadata

@ivaaaan ivaaaan closed this as completed Nov 8, 2023
@ivaaaan ivaaaan reopened this Nov 8, 2023
@arthurspa
Copy link

What about exposing message.Context() to the log method? Would it be possible?
This way we could inject the correlation ID (or anything else to the message's context) in the middleware and the logger implementation would use it to customise the logger fields.

@bastiankoetsier
Copy link

Hey, I just found this thread evaluating possible ways handling an error on the router level. I do wonder if it make sense to have a "router-global error handler" (for the lack of better terms) which could fall back to the existing default behaviour. This way, you can be totally flexible how you want to handle the error in general; you could just send it to an error tracker instead of plain logging. Using a middleware for this feels odd as it needs to swallow the handler-error at the end to keep the semantics. Would you be open to have a PR for that @roblaszczak @m110 ?
(sorry to hijack this thread @ivaaaan 😅 )

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

3 participants