Skip to content

Conversation

@eLud
Copy link

@eLud eLud commented Nov 27, 2025

Fix: Notify caller when channel becomes inactive unexpectedly

Problem

When network connectivity is lost (e.g., 100% packet loss), requests hang indefinitely instead of timing out or returning an error. This is a regression
introduced in v1.1.0.

Potential fix for #361

Root Cause

When network drops, NIO's HTTP/2 layer detects the connection loss and triggers channelInactive before the IdleStateHandler can fire its timeout event.
However, channelInactive only closed the connection without notifying the caller:

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

This worked in v1.0.4 because the retain cycle (fixed in #354) kept handlers alive longer, giving IdleStateHandler time to fire. With the fix, cleanup happens faster and channelInactive wins the race—but doesn't notify the caller.

Solution

  1. Added a hasResponded flag to track whether the response callback has been invoked
  2. Modified channelInactive to call the response callback with .unavailable error if no response was received yet
  func channelInactive(context: ChannelHandlerContext) {
      let shouldNotify = !self.hasResponded
      self.closeConnection()
      if shouldNotify {
          self.hasResponded = true
          self.onResponse(.init(
              code: .unavailable,
              error: ConnectError(code: .unavailable, message: "Channel became inactive", ...)
          ))
      }
      context.fireChannelInactive()
  }

Changes

  • ConnectUnaryChannelHandler.swift: Added hasResponded flag and updated channelInactive
  • ConnectStreamChannelHandler.swift: Added hasResponded flag and updated channelInactive

Testing

Tested with Network Link Conditioner at 100% packet loss:

  • Before: Request hangs indefinitely
  • After: Request returns .unavailable error promptly

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

Successfully merging this pull request may close these issues.

1 participant