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

Prelude.undefined called from Network.HTTP2.Client.Run.hs:154:30 #139

Closed
FinleyMcIlwaine opened this issue Jul 26, 2024 · 6 comments
Closed

Comments

@FinleyMcIlwaine
Copy link
Collaborator

It seems the new exception handling behavior introduced in #137 has uncovered a previously unreachable path. I'm seeing Prelude.undefined errors during some of the stress tests for our gRPC library

test-stress: Prelude.undefined
CallStack (from HasCallStack):
  undefined, called at ./Network/HTTP2/Client/Run.hs:154:30 in http2-5.3.1-95e9324b:Network.HTTP2.Client.Run
@edsko
Copy link
Collaborator

edsko commented Jul 26, 2024

I suspect this is not directly related to #137 (perhaps it made some non-determinism happen slightly differently). We're seeing this call to undefined when the server dies.

@edsko
Copy link
Collaborator

edsko commented Jul 26, 2024

I think the problem is that the background threads are terminating normally (without an exception) when the server dies (maybe only sometimes?).

@kazu-yamamoto
Copy link
Owner

The background threads (receiver, sender, etc) can finish earlier than client.
For instance, the server can send GOAWAY.
We need to decide how to treat this situation and then replace the undefined with proper code.
Any opinion?

@edsko
Copy link
Collaborator

edsko commented Sep 11, 2024

Funny that you are looking at this again, we've been looking at this also :) We think the fix isn't hard, but we hadn't had the time yet to take a look, hopefully soon (this week). PR will be coming soon!

@kazu-yamamoto
Copy link
Owner

Thanks in advance.

@kazu-yamamoto
Copy link
Owner

GOAWAY contains a stream ID.
This is important information for clients because streams whose stream ID is larger than this value can be resent safely in a new connection.
So, I guess that a client should received this value from the background threads.

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

No branches or pull requests

3 participants