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.

Sign up for GitHub

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

Jump to bottom

call_all should stop after poll_ready returns an error #706

Closed
leoyvens opened this issue Oct 26, 2022 · 1 comment · Fixed by #709
Closed

call_all should stop after poll_ready returns an error #706

leoyvens opened this issue Oct 26, 2022 · 1 comment · Fixed by #709
Labels
A-util Area: The tower "util" module C-bug Category: This is a bug. E-help-wanted Call for participation: Help is requested to fix this issue.

Comments

@leoyvens
Copy link
Contributor

The docs say that a service should be discarded after an error is returned from poll_ready. But CallAll will currently treat a poll_ready error in the same way as a request error and continues to call the service. It seems to me that CallAll should instead return the error once and end the stream even if there are more requests to be made.
@LucioFranco
Copy link
Member

Yeah, that seems right to me, I would accept a PR for this!

@LucioFranco LucioFranco added C-bug Category: This is a bug. E-help-wanted Call for participation: Help is requested to fix this issue. A-util Area: The tower "util" module labels Oct 26, 2022
leoyvens added a commit to leoyvens/tower that referenced this issue Nov 1, 2022
@leoyvens
util: Two call_all bug fixes
e81a83e 
One is handling poll_ready errors (tower-rs#706).

The other is fixing the TODO about disarming poll_ready, since there is no disarm this makes sure
`poll_ready` is only called if `call` will immediately follow.
leoyvens added a commit to leoyvens/tower that referenced this issue Nov 1, 2022
@leoyvens
util: Two call_all bug fixes
e8c461a 
One is handling poll_ready errors (tower-rs#706).

The other is fixing the TODO about disarming poll_ready, since there is no disarm this makes sure
`poll_ready` is only called if `call` will immediately follow.
@leoyvens leoyvens mentioned this issue Nov 1, 2022
Merged
leoyvens added a commit to leoyvens/tower that referenced this issue Nov 1, 2022
@leoyvens
util: Two call_all bug fixes
3ee139a 
One is handling poll_ready errors (tower-rs#706).

The other is fixing the TODO about disarming poll_ready, since there is no disarm this makes sure
`poll_ready` is only called if `call` will immediately follow.
leoyvens added a commit to leoyvens/tower that referenced this issue Nov 1, 2022
@leoyvens
util: Two call_all bug fixes
6ea1d23 
One is handling poll_ready errors (tower-rs#706).

The other is fixing the TODO about disarming poll_ready, since there is no disarm this makes sure
`poll_ready` is only called if `call` will immediately follow.
LucioFranco pushed a commit that referenced this issue Nov 1, 2022
@leoyvens
util: Two call_all bug fixes (#709)
c9d84cd 
One is handling poll_ready errors (#706).

The other is fixing the TODO about disarming poll_ready, since there is no disarm this makes sure
`poll_ready` is only called if `call` will immediately follow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-util Area: The tower "util" module C-bug Category: This is a bug. E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

util: Two call_all bug fixes leoyvens/tower
2 participants
@LucioFranco @leoyvens