Skip to content

RELAY: RelayClient overwrites the live Call instance on a redelivered calling.call.receive, orphaning the app's call handler #97

Description

@jpsantosbh

Describe the bug

RelayClient._handle_inbound_call unconditionally constructs a new Call and writes it into self._calls under the incoming call_id, with no check for an entry already there. When RELAY redelivers calling.call.receive for a call that is already in flight, the live Call is evicted from the map and the application is left holding an object that no longer receives events.

Consequences:

  • Every later event for that call_id routes to the replacement instance, so the Call the application is holding goes silent.
  • An awaited call.connect() / call.play() / call.record() on the original object never resolves — it hangs to its timeout instead of returning at hangup.
  • The on_call handler is entered a second time, with a different Call, for what is one call.

signalwire/signalwire/relay/client.py @ 8253718:

async def _handle_inbound_call(self, payload: dict[str, Any]) -> None:
    if len(self._calls) >= self._max_active_calls:      # :1052 — runs before any dedup
        ...
        return
    ...
    self._calls[call_id] = call                          # :1073 — no `call_id in self._calls` guard

    if self._on_call_handler:
        self._spawn_bg(self._safe_call_handler(call))    # :1076 — unconditionally re-entered

The dial path one branch away already does the right thing — client.py:1032-1037 guards _register_dial_leg with exactly the missing check:

if (tag and tag in self._pending_dials and call_id
        and call_id not in self._calls):
    self._register_dial_leg(tag, event_params)

Routing (client.py:1040-1046) only ever reads self._calls.get(call_id), so once the entry is overwritten the first Call is unreachable and is never told.

To reproduce

Against the repo's existing connected_client / AutoAuthMockWebSocket fixtures:

@pytest.mark.asyncio
async def test_redelivered_receive_is_idempotent(connected_client) -> None:
    client, ws = connected_client
    handler_calls: list[Call] = []

    @client.on_call
    async def handle(call: Call) -> None:
        handler_calls.append(call)

    def receive_event() -> dict:
        return make_event(EVENT_CALL_RECEIVE, {
            "call_id": "c1", "node_id": "n1", "project_id": "test-project",
            "direction": "inbound", "call_state": "ringing",
        })

    ws.feed_message(receive_event())
    await asyncio.sleep(0.05)
    first = handler_calls[0]

    # RELAY redelivers the same receive for the same call_id.
    ws.feed_message(receive_event())
    await asyncio.sleep(0.05)

    assert len(handler_calls) == 1          # handler must not be re-entered
    assert client._calls["c1"] is first     # live instance must survive

    ws.feed_message(make_event(EVENT_CALL_STATE, {"call_id": "c1", "call_state": "answered"}))
    await asyncio.sleep(0.05)
    assert first.state == "answered"        # original must still receive events

Observed today:

HANDLER_ENTRIES 2   SAME_INSTANCE False   MAP_IS_FIRST False
FIRST_STATE ringing
AssertionError: assert 2 == 1

The handler runs twice with distinct Call objects, _calls["c1"] holds the replacement, and the original is frozen at ringing.

Expected behavior

  1. calling.call.receive should be idempotent per call_id. If call_id in self._calls, keep the existing instance instead of constructing and storing a replacement.
  2. A redelivery for a call already in flight should not re-enter _on_call_handler. A redelivery is a transport-level retry, not a second call, and the SDK is the right layer to absorb it — otherwise every application has to write the workaround itself.
  3. An application should never end up holding a Call that silently receives no further events. Reusing the instance satisfies this by construction; any other fix has to notify the orphan so the application fails fast instead of hanging to timeout.

Two details worth carrying over from the TypeScript fix:

  • The _max_active_calls check should move below the dedup. It currently runs first, so a redelivery arriving at capacity is counted as a new call and logged as a drop.
  • ACKing is unaffected either way — the event is ACKed before _handle_event runs, so an early return still tells the server the redelivery was consumed and stops the retries.

Additional context

This was reported against the TypeScript SDK (@signalwire/sdk 2.0.5) and fixed there in signalwire/signalwire-typescript#182, which lands as 2.0.6 on the 2.0.x line. While verifying that report I checked this port and found the identical defect — same unconditional map write, same unguarded handler spawn, same asymmetry with the dial path. Filing it here so a TypeScript-only patch doesn't leave Python users hanging on the same timeout; the TS PR is a usable reference implementation.

Internal tracker: signalwire/cloud-product#20481 (private).

Until this is fixed, applications can absorb the redelivery themselves:

calls_in_flight: dict[str, Call] = {}

@client.on_call
async def handle(call: Call) -> None:
    in_flight = calls_in_flight.get(call.call_id)
    if in_flight is not None:
        client._calls[call.call_id] = in_flight   # restore the original instance
        return
    calls_in_flight[call.call_id] = call
    try:
        await on_call_received(call)
    finally:
        calls_in_flight.pop(call.call_id, None)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions