Skip to content

Comments

Fixed rewriting the logger in logging middleware#8

Merged
miladz68 merged 1 commit intoCoreumFoundation:masterfrom
miladz68:milad/fix/not-rewrite-logger-middleware
Sep 5, 2022
Merged

Fixed rewriting the logger in logging middleware#8
miladz68 merged 1 commit intoCoreumFoundation:masterfrom
miladz68:milad/fix/not-rewrite-logger-middleware

Conversation

@miladz68
Copy link
Contributor

@miladz68 miladz68 commented Sep 2, 2022

the logger gets rewritten on the middleware which is not correct, it should be copied and modified.


This change is Reviewable

Copy link
Collaborator

@wojtek-coreum wojtek-coreum left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @begmaroman, @dzmitryhil, and @ysv)

Copy link
Collaborator

@ysv ysv left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @begmaroman, @dzmitryhil, and @miladz68)


http/errors.go line 25 at r1 (raw file):

			err := next(c)
			if err != nil {
				logger := withEchoContext(logger, c)

that is why I always insist on not having name collisions with neither other variables or packages

non-blocking:
but can you please rename the var to smth different ?

@miladz68 miladz68 merged commit d98640b into CoreumFoundation:master Sep 5, 2022
@miladz68 miladz68 deleted the milad/fix/not-rewrite-logger-middleware branch September 5, 2022 06:42
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.

3 participants