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

Backport: Log requests with empty body #1861

Merged
merged 1 commit into from May 17, 2018

Conversation

@rossabaker
Copy link
Member

@rossabaker rossabaker commented May 14, 2018

Backports #1852. I didn't look carefully at the target branch before I merged.

@rossabaker rossabaker added the bug label May 14, 2018
@rossabaker
Copy link
Member Author

@rossabaker rossabaker commented May 14, 2018

Hmm. I bet this still drops the request log when the response body is None.

Loading

@ChristopherDavenport
Copy link
Member

@ChristopherDavenport ChristopherDavenport commented May 14, 2018

I think we don't have a clear answer on when request and response both have empty bodies that aren't read.

Loading

Copy link
Member

@ChristopherDavenport ChristopherDavenport left a comment

👍

Loading

@rossabaker
Copy link
Member Author

@rossabaker rossabaker commented May 14, 2018

This moves the finalizer to the response, which might not exist. I think we can plug that hole with an orElse case. In the HttpApp world, this isn't a problem. I think it's a bug in this backport.

Loading

@rossabaker
Copy link
Member Author

@rossabaker rossabaker commented May 14, 2018

Hmm. We're only logging responses on the happy path. Nothing gets logged if the service raises an error or returns None.

Loading

@rossabaker rossabaker dismissed ChristopherDavenport’s stale review May 16, 2018

I'm going to fix the other bugs as part of this.

@rossabaker
Copy link
Member Author

@rossabaker rossabaker commented May 17, 2018

@lucaviolanti just hit this. I'm going to merge this and do the other fixes in another PR.

Loading

@rossabaker rossabaker merged commit 7cd7dd5 into http4s:release-0.18.x May 17, 2018
1 of 2 checks passed
Loading
@rossabaker rossabaker deleted the backport-1852 branch May 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants