Skip to content

Conversation

@ChiragAgg5k
Copy link
Member

@ChiragAgg5k ChiragAgg5k commented Oct 24, 2025

Problem

The onOpen callback was not being triggered when a WebSocket connection was established in the Swift WebSocket client. This affected all SDKs that depend on this implementation, including the Apple (iOS/macOS) SDK.

Root Cause

The issue was caused by a lifecycle timing problem in the NIO channel handler pipeline:

  1. HTTP channel becomes active first - When the initial connection is made, the channel's channelActive event fires for existing handlers
  2. HTTP-to-WebSocket upgrade occurs - The protocol upgrade negotiation happens via HTTP
  3. MessageHandler is added to pipeline - Only after successful upgrade is the MessageHandler added to the pipeline
  4. channelActive never fires - Since the handler was added to an already-active channel, its channelActive method never gets called

The channelActive callback only fires when a handler is added to a channel that is not yet active. By the time our MessageHandler is added, the channel is already active from the HTTP connection phase.

Solution

The fix explicitly triggers the onOpen callback in the upgradePipelineHandler method after successfully adding the MessageHandler to the pipeline:

return channel.pipeline.addHandler(handler).map {
    // Manually trigger onOpen since channelActive was already called before this handler was added
    if let delegate = self.delegate {
        delegate.onOpen(channel: channel)
    } else {
        self.onOpen(channel)
    }
}

This ensures the callback is invoked at the correct time - when the WebSocket connection is fully established and ready to send/receive messages.

Impact

This fix ensures that:

  • onOpen callbacks work correctly in the Swift WebSocket client
  • Apple SDK's Realtime service properly receives connection open events
  • Heartbeat mechanisms and other connection-dependent features activate correctly
  • Any application code relying on the onOpen callback now functions as expected

Testing

Tested with Apple SDK Realtime service which uses this WebSocket client as a dependency.

Summary by CodeRabbit

  • Bug Fixes
    • WebSocket upgrade events now correctly invoke callbacks to notify listeners of successful connection establishment. Delegates receive proper notifications when configured, ensuring connection state is accurately communicated.

Previously, the onOpen callback was not being triggered when a WebSocket
connection was established. This was due to a lifecycle timing issue where
the MessageHandler's channelActive method was never called.

The issue occurred because:
1. HTTP channel becomes active first
2. HTTP-to-WebSocket upgrade happens
3. MessageHandler is added to the pipeline after the channel is already active
4. channelActive only fires when a handler is added to an inactive channel

The fix explicitly triggers the onOpen callback in the upgradePipelineHandler
method after successfully adding the MessageHandler to the pipeline, which is
exactly when the WebSocket connection is fully established.

This ensures the onOpen callback works correctly in all SDKs that depend on
this Swift WebSocket client, including the Apple (iOS/macOS) SDK.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 24, 2025

Caution

Review failed

The head commit changed during the review from f1fa1b3 to 85fb57a.

Walkthrough

The change modifies the WebSocket upgrade pipeline handler in the Swift client template to chain a map operation that invokes an onOpen callback upon successful pipeline upgrade. When invoked, the handler checks if a delegate is set and calls its onOpen method; otherwise, it falls back to invoking an internal onOpen method. This addition executes a side-effect during upgrade while maintaining backward compatibility with existing behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

The modification is a localized, single-file change that introduces a straightforward callback invocation pattern. The logic is linear with a simple conditional check between delegate and internal method calls. The scope is contained within the pipeline upgrade handler, and the pattern adds minimal complexity without introducing multi-faceted considerations.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "fix: trigger onOpen callback in Swift WebSocket after upgrade completes" directly and clearly describes the main change in the changeset. The PR implements a fix that explicitly invokes the onOpen callback after the MessageHandler is added to the channel pipeline during WebSocket upgrade, which is exactly what the title conveys. The title is specific (naming the callback and context), concise, and easily understandable to someone reviewing the commit history.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ChiragAgg5k ChiragAgg5k requested a review from Copilot October 24, 2025 10:51
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug where the onOpen callback was not being triggered in the Swift WebSocket client due to a channel lifecycle timing issue. The handler was being added to an already-active channel after the HTTP-to-WebSocket upgrade, so channelActive never fired for the MessageHandler.

Key Changes:

  • Explicitly invokes the onOpen callback after adding the MessageHandler to the pipeline
  • Ensures connection open events are properly communicated to delegates or the client itself

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@ChiragAgg5k ChiragAgg5k force-pushed the fix-swift-websocket-onopen-callback branch from f1fa1b3 to 85fb57a Compare October 24, 2025 11:00
@stnguyen90 stnguyen90 requested a review from abnegate October 24, 2025 15:58
@ChiragAgg5k ChiragAgg5k merged commit b4a2fd9 into master Oct 28, 2025
133 of 137 checks passed
@ChiragAgg5k ChiragAgg5k deleted the fix-swift-websocket-onopen-callback branch October 28, 2025 04:55
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.

3 participants