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

fix: middleware causes crash #15

Merged
merged 1 commit into from
Nov 24, 2023
Merged

fix: middleware causes crash #15

merged 1 commit into from
Nov 24, 2023

Conversation

niklastreml
Copy link
Collaborator

@niklastreml niklastreml commented Nov 24, 2023

Motivation

Using the logger middleware causes the application to crash.

Fix

Take the logger out of the context and write it into the request context. The issue was, that we were rewriting the request context with our context, which essentially removes any context added by chi. This causes an issue internally when chi tries to retrieve the request context to read some of its own injected metadata. The key for this does not exist due to us overwriting it, so the following type assertion fails, since chi is essentially doing this reqCtx := nil.(RequestContext) without an ok check, causing a panic.

@niklastreml niklastreml added the bug Something isn't working label Nov 24, 2023
@niklastreml niklastreml requested a review from y-eight November 24, 2023 09:44
@niklastreml niklastreml self-assigned this Nov 24, 2023
Copy link
Member

@y-eight y-eight left a comment

Choose a reason for hiding this comment

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

perfect 👌 thx

@niklastreml niklastreml merged commit 2fadcf4 into main Nov 24, 2023
1 check passed
@niklastreml niklastreml deleted the fix/middleware-crash branch November 24, 2023 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants