Unreliable timeout in 1.1.0

[eLud]

I recently updated the Connect lib to 1.1.0 in an app, and was able to notice that the timeout specified in ProtocolClient and ProtocolClientConfig seems to not be working reliably. Are you aware of this?

I can't reproduce every time, but was still able to do it multiple times + had user feedback about it.

Step to reproduce:

• Set a timeout
• Set Network Link Conditioner on, with 100% Loss config
• Trigger the request

Outcome:

• The request never gets an answer, and Connect never didn't time out either.
• If you are awaiting for the result in a specific loading state, the app get stuck in this state

Expected behavior:

• The timeout fires consistently if no response is received after the specified time.

[aymericbeaumet]

@rebello95: do you have any insights on this? I'm wondering if this is only the top of the iceberg with potentially deeper ramifications.

[rebello95]

👋🏽 hey @aymericbeaumet and @eLud! Apologies - I did see this issue and thought I responded, but must've failed to save my comment 😓

I spent a bit of time trying to reproduce this but wasn't able to using the config in the screenshot above. Do either of you have tips for reproducing this or sample projects where it's happening for you consistently?

cc @eseay in case you've come across this as well

[eLud]

Sorry for the late answer, I was not able to come back to this sooner. I updated back Connect to 1.2.0 lately (from 1.0.4 where we had no issues), and faced again a case where a user was stuck indefinitely because a network call was not timing out.

I don't have tried to reproduce it consistently yet, but I'll see if I can provide you with something.

[eLud]

Possible Root Cause Found

After adding debug logging, with an LLM driven investigation, I might have identified the issue. The proposed fix indeed fix the issue I experienced (basically, starting our app locally with local backend stoped and try to sent a request to it.

Before: No callback / timeout
After: Properly getting an error

Error details:
→ type: ConnectError
→ message: ConnectError(code: Connect.Code.unavailable, message: Optional("Channel became inactive"), exception: nil, details: [], metadata: [:])
→ stack: ConnectError(code: Connect.Code.unavailable, message: Optional("Channel became inactive"), exception: nil, details: [], metadata: [:])

Does it sound correct to you? If so, I can provide a fix PR.

What happens when network is unavailable

📴 [ConnectUnary] channelInactive called
🔒 [ConnectUnary] closeConnection called, isClosed: false, context nil: false
⚠️ [ConnectUnary] handlerRemoved called

The IdleStateHandler timeout never fires. Instead, NIO's HTTP/2 layer detects the connection loss and triggers channelInactive first.

The bug

In ConnectUnaryChannelHandler.swift, channelInactive closes the channel but never calls onResponse:

func channelInactive(context: ChannelHandlerContext) {
    self.closeConnection()
    context.fireChannelInactive()
    // ❌ onResponse is never called - request hangs forever
}

Compare to the timeout handler which correctly notifies the caller:

func userInboundEventTriggered(context: ChannelHandlerContext, event: Any) {
    // ...
    self.closeConnection()
    self.onResponse(.init(code: .deadlineExceeded, ...))  // ✅ Caller is notified
}

Why 1.0.4 worked

The retain cycle (fixed in PR #354) likely kept the handler alive longer, giving IdleStateHandler time to fire before the channel went inactive. With the
fix, cleanup happens faster, and channelInactive wins the race—but doesn't notify the caller.

Proposed fix

channelInactive should call onResponse with an error if no response was received:

func channelInactive(context: ChannelHandlerContext) {
    let shouldNotify = !self.isClosed
    self.closeConnection()
    if shouldNotify {
        self.onResponse(.init(
            code: .unavailable,
            headers: [:],
            message: nil,
            trailers: [:],
            error: ConnectError(
                code: .unavailable,
                message: "Connection closed unexpectedly",
                exception: nil,
                details: [],
                metadata: [:]
            ),
            tracingInfo: nil
        ))
    }
    context.fireChannelInactive()
}

The same fix should be applied to ConnectStreamChannelHandler.swift.

[rebello95]

@eLud thank you for the analysis above and for putting up a potential fix in #376! I left a comment on your PR