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 response body abort warning #280

Merged
merged 2 commits into from
Dec 29, 2020
Merged

Fix response body abort warning #280

merged 2 commits into from
Dec 29, 2020

Conversation

sagebind
Copy link
Owner

@sagebind sagebind commented Dec 21, 2020

Previously we were using two different atomic sources to discover when a response has been dropped, but only emitting a warning for an unconsumed response body if one of the sources indicated a drop. This was causing inconsistent results of when the warning would be emitted. Fix this by having just one source of truth for when the response is dropped.

In addition, change the warning log into an info log instead with a better message. Aborting a response stream is not incorrect and may be intentionally desired by the user. After #257 we could directly suggest using consume() in the log message.

See #270.

@codecov
Copy link

codecov bot commented Dec 21, 2020

Codecov Report

Merging #280 (2230e61) into master (cb5b36c) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #280      +/-   ##
==========================================
- Coverage   72.68%   72.65%   -0.03%     
==========================================
  Files          51       51              
  Lines        2738     2732       -6     
==========================================
- Hits         1990     1985       -5     
+ Misses        748      747       -1     
Impacted Files Coverage Δ
src/handler.rs 71.10% <100.00%> (-0.33%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb5b36c...2230e61. Read the comment docs.

Previously we were using two different atomic sources to discover when a response has been dropped, but only emitting a warning for an unconsumed response body if one of the sources indicated a drop. This was causing inconsistent results of when the warning would be emitted. Fix this by having just one source of truth for when the response is dropped.

In addition, change the warning log into an info log instead with a better message. Aborting a response stream is not incorrect and may be intentionally desired by the user. After #257 we could directly suggest using `consume()` in the log message.

See #270.
@sagebind sagebind force-pushed the abort-connection-warning branch from 0e0507c to 11456e0 Compare December 29, 2020 06:39
@sagebind sagebind changed the title Fix abort warning and add explicit abort method Fix response body abort warning Dec 29, 2020
@sagebind sagebind merged commit 048e895 into master Dec 29, 2020
@sagebind sagebind deleted the abort-connection-warning branch December 29, 2020 18:02
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.

1 participant