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)

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