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

Consider removing "dispatch dropped without returning error" #2649

Closed
nox opened this issue Sep 16, 2021 · 2 comments
Closed

Consider removing "dispatch dropped without returning error" #2649

nox opened this issue Sep 16, 2021 · 2 comments
Labels
A-client Area: client. C-feature Category: feature. This is adding a new feature. E-medium Effort: medium. Some knowledge of how hyper internal works would be useful.

Comments

@nox
Copy link
Contributor

nox commented Sep 16, 2021

Is your feature request related to a problem? Please describe.
Hyper panics with "dispatch dropped without returning error" if the background dispatch task goes away, either because the runtime was dropped or some user-provided code such as a HttpBody implementation panicked

Describe the solution you'd like
Hyper should catch the panic and return a proper error instead of panicking with "dispatch dropped without returning error".

Cc @seanmonstar

@nox nox added the C-feature Category: feature. This is adding a new feature. label Sep 16, 2021
@seanmonstar
Copy link
Member

Yea, we can do better. We probably can't completely remove the panic, since the Canceled error is part of the oneshot contract. But we can improve two the existing cases that can happen, to try to eliminate it so that panic never can happen.

  • The runtime is dropped, killing the dispatch task.
  • The dispatch task panics, due to user code.

We can add a Guard to the oneshot::Sender (Callback) to make it send an appropriate hyper::Error in drop, if not consumed. If std::thread::panicking(), then we can send an error about the dispatch task having panicked. If not, we can send an error about an unexpected runtime dropping the task.

@seanmonstar seanmonstar added A-client Area: client. E-medium Effort: medium. Some knowledge of how hyper internal works would be useful. labels Sep 16, 2021
@jyn514
Copy link
Contributor

jyn514 commented Feb 7, 2022

I'm seeing this panic in real code, it would be nice to avoid it somehow. Not sure how it happens.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-client Area: client. C-feature Category: feature. This is adding a new feature. E-medium Effort: medium. Some knowledge of how hyper internal works would be useful.
Projects
None yet
Development

No branches or pull requests

3 participants