search

kazu-yamamoto / http2

HTTP/2.0 library including HPACK
BSD 3-Clause "New" or "Revised" License
83 stars 21 forks source link

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

Open FinleyMcIlwaine opened 1 month ago

FinleyMcIlwaine commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

kazu-yamamoto commented 2 days ago

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 commented 2 days ago

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 commented 2 days ago

Thanks in advance.

kazu-yamamoto commented 2 days ago

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.