Skip to content

Fix: Correct Client State Management in LaravelHttpTransport #17

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

Merged
merged 1 commit into from
Jun 13, 2025

Conversation

CodeWithKyrian
Copy link
Contributor

This PR addresses a bug in LaravelHttpTransport that caused issues with tracking active clients, leading to "inactive or unknown client" errors when attempting to send messages.

Problem:

The LaravelHttpTransport previously maintained an internal, in-memory $activeClients array. In a standard Laravel request-response environment (without persistent workers like Octane), this transport instance is re-created for almost every HTTP interaction (initial SSE GET, message POSTs, SSE stream polls). This meant the in-memory $activeClients list was unreliable and often out of sync with the actual client state managed by ClientStateManager (which uses a persistent cache).

As a result, even if ClientStateManager knew a client was active (e.g., after client_connected was emitted in the SSE GET request), a subsequent sendToClientAsync call triggered by a server-side event might find the $activeClients array empty in the new transport instance handling that server-side logic, leading to failed message delivery.

Solution:

  1. Removed the internal $activeClients array from LaravelHttpTransport.
  2. Removed the client_connected and client_disconnected event listeners within LaravelHttpTransport that were managing this local array. Client connection and disconnection events are still emitted for the Protocol layer to handle with ClientStateManager.
  3. LaravelHttpTransport::sendToClientAsync now directly queues messages via ClientStateManager without checking the local $activeClients array.
  4. The existing message event listener in LaravelHttpTransport that calls clientStateManager->updateClientActivity() remains, as this is the correct way to signal activity to the persistent state manager.

Closes #13

Removes in-memory active client tracking (`$activeClients` array and
associated event listeners) from `LaravelHttpTransport`. This local tracking
was unreliable due to Laravel's typical request-response lifecycle where
the transport instance is re-initialized per interaction.

The `ClientStateManager`, which uses a persistent cache, is already
responsible for managing active client state and activity across requests.
The `LaravelHttpTransport` now correctly relies solely on `ClientStateManager`
for queuing messages (`sendToClientAsync`) and updates client activity
via `ClientStateManager` when messages are received.
@CodeWithKyrian CodeWithKyrian merged commit cdf7fba into main Jun 13, 2025
8 checks passed
@CodeWithKyrian CodeWithKyrian deleted the fix/laravel-transport-state branch June 13, 2025 07:02
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.

Error: Not actively managed by this transport
1 participant