fix(server): scope accept-rollback terminal to the acceptor only
Code-review catch: the Path-2 accept-rollback previously emitted
dm_call_undeliverable { terminal: true } via sendToFederatedCallUsers,
which broadcasts to every ringedUserIds entry. In a group DM this
would prematurely tear down non-accepting ringees whose own accept /
reject / timeout paths should govern their state. Switch to
sendToUser(acceptorId) so only the acting user gets the terminal
signal. Reorder the clearFederatedCall to happen before the emit so a
concurrent end-handler sees a cleared entry (clearFederatedCall is
idempotent). Spec updated, test extended to assert the scoping with a
two-ringee group-DM fixture.
This commit is contained in:
@@ -234,13 +234,19 @@ describe('ring-timeout fan-out hook', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe('handleDmCallAccept Path-2 relay failure', () => {
|
describe('handleDmCallAccept Path-2 relay failure', () => {
|
||||||
it('emits dm_call_undeliverable { phase:"accept", terminal:true } and clears the fedCall when the relay fails', async () => {
|
it('emits dm_call_undeliverable { phase:"accept", terminal:true } to the acceptor ONLY and clears the fedCall when the relay fails', async () => {
|
||||||
const { handleDmCallAcceptForTest } = await importSUT();
|
const { handleDmCallAcceptForTest } = await importSUT();
|
||||||
const connectionManager = await importManager();
|
const connectionManager = await importManager();
|
||||||
|
|
||||||
const fedCall = makeFedCall();
|
const fedCall = makeFedCall({
|
||||||
|
// Two ringed users — a group DM where only one accepts. Only the acceptor
|
||||||
|
// should receive the terminal undeliverable; the other ringee must stay
|
||||||
|
// in ring state so their own timeout/reject path governs teardown.
|
||||||
|
ringedUserIds: ['acceptor-user', 'other-ringee'],
|
||||||
|
});
|
||||||
connectionManager.createFederatedCall(fedCall);
|
connectionManager.createFederatedCall(fedCall);
|
||||||
const sendToUsersSpy = vi.spyOn(connectionManager, 'sendToFederatedCallUsers');
|
const sendToUserSpy = vi.spyOn(connectionManager, 'sendToUser');
|
||||||
|
const sendToCallUsersSpy = vi.spyOn(connectionManager, 'sendToFederatedCallUsers');
|
||||||
sendCallRelayMock.mockResolvedValue({ ok: false, reason: 'peer_transient_failure', error: 'timeout' });
|
sendCallRelayMock.mockResolvedValue({ ok: false, reason: 'peer_transient_failure', error: 'timeout' });
|
||||||
|
|
||||||
await handleDmCallAcceptForTest(
|
await handleDmCallAcceptForTest(
|
||||||
@@ -250,14 +256,23 @@ describe('handleDmCallAccept Path-2 relay failure', () => {
|
|||||||
);
|
);
|
||||||
|
|
||||||
expect(connectionManager.getFederatedCall(fedCall.federatedId)).toBeUndefined();
|
expect(connectionManager.getFederatedCall(fedCall.federatedId)).toBeUndefined();
|
||||||
const calls = sendToUsersSpy.mock.calls.filter(([, ev]) =>
|
|
||||||
|
// Terminal undeliverable went to the acceptor only.
|
||||||
|
const undelivCalls = sendToUserSpy.mock.calls.filter(([, ev]) =>
|
||||||
(ev as { type: string }).type === 'dm_call_undeliverable',
|
(ev as { type: string }).type === 'dm_call_undeliverable',
|
||||||
);
|
);
|
||||||
expect(calls).toHaveLength(1);
|
expect(undelivCalls).toHaveLength(1);
|
||||||
const undeliverable = calls[0]![1] as { phase: string; terminal: boolean; failures: unknown[] };
|
expect(undelivCalls[0]![0]).toBe('acceptor-user');
|
||||||
|
const undeliverable = undelivCalls[0]![1] as { phase: string; terminal: boolean; failures: unknown[] };
|
||||||
expect(undeliverable.phase).toBe('accept');
|
expect(undeliverable.phase).toBe('accept');
|
||||||
expect(undeliverable.terminal).toBe(true);
|
expect(undeliverable.terminal).toBe(true);
|
||||||
expect(undeliverable.failures).toHaveLength(1);
|
expect(undeliverable.failures).toHaveLength(1);
|
||||||
|
|
||||||
|
// No undeliverable broadcast to all ringed users.
|
||||||
|
const callUsersCalls = sendToCallUsersSpy.mock.calls.filter(([, ev]) =>
|
||||||
|
(ev as { type: string }).type === 'dm_call_undeliverable',
|
||||||
|
);
|
||||||
|
expect(callUsersCalls).toHaveLength(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('does not emit undeliverable on relay success', async () => {
|
it('does not emit undeliverable on relay success', async () => {
|
||||||
@@ -266,7 +281,7 @@ describe('handleDmCallAccept Path-2 relay failure', () => {
|
|||||||
|
|
||||||
const fedCall = makeFedCall();
|
const fedCall = makeFedCall();
|
||||||
connectionManager.createFederatedCall(fedCall);
|
connectionManager.createFederatedCall(fedCall);
|
||||||
const sendToUsersSpy = vi.spyOn(connectionManager, 'sendToFederatedCallUsers');
|
const sendToUserSpy = vi.spyOn(connectionManager, 'sendToUser');
|
||||||
sendCallRelayMock.mockResolvedValue({ ok: true });
|
sendCallRelayMock.mockResolvedValue({ ok: true });
|
||||||
|
|
||||||
await handleDmCallAcceptForTest(
|
await handleDmCallAcceptForTest(
|
||||||
@@ -275,7 +290,7 @@ describe('handleDmCallAccept Path-2 relay failure', () => {
|
|||||||
{} as never,
|
{} as never,
|
||||||
);
|
);
|
||||||
|
|
||||||
const undelivCalls = sendToUsersSpy.mock.calls.filter(([, ev]) =>
|
const undelivCalls = sendToUserSpy.mock.calls.filter(([, ev]) =>
|
||||||
(ev as { type: string }).type === 'dm_call_undeliverable',
|
(ev as { type: string }).type === 'dm_call_undeliverable',
|
||||||
);
|
);
|
||||||
expect(undelivCalls).toHaveLength(0);
|
expect(undelivCalls).toHaveLength(0);
|
||||||
|
|||||||
@@ -1563,7 +1563,12 @@ async function handleDmCallAccept(event: Record<string, unknown>, userId: string
|
|||||||
if (!result.ok) {
|
if (!result.ok) {
|
||||||
console.error(`[federation] dm_call_accept relay to ${fedCall.federatedCallHost} failed (${result.reason}): ${result.error}`);
|
console.error(`[federation] dm_call_accept relay to ${fedCall.federatedCallHost} failed (${result.reason}): ${result.error}`);
|
||||||
const failure = buildFailureFromResult(result, fedCall.federatedCallHost, db);
|
const failure = buildFailureFromResult(result, fedCall.federatedCallHost, db);
|
||||||
connectionManager.sendToFederatedCallUsers(fedCall.federatedId, {
|
// Clear first so a concurrent end-handler sees a cleared entry (idempotent).
|
||||||
|
connectionManager.clearFederatedCall(fedCall.federatedId);
|
||||||
|
// Terminal targets ONLY the acceptor — other ringed users (group DM) didn't
|
||||||
|
// accept and should stay in their ring state; their own dm_call_end / timeout
|
||||||
|
// paths govern their teardown.
|
||||||
|
connectionManager.sendToUser(userId, {
|
||||||
type: 'dm_call_undeliverable',
|
type: 'dm_call_undeliverable',
|
||||||
dmChannelId: fedCall.dmChannelId,
|
dmChannelId: fedCall.dmChannelId,
|
||||||
federatedCallId: fedCall.federatedId,
|
federatedCallId: fedCall.federatedId,
|
||||||
@@ -1571,7 +1576,6 @@ async function handleDmCallAccept(event: Record<string, unknown>, userId: string
|
|||||||
phase: 'accept',
|
phase: 'accept',
|
||||||
failures: [failure],
|
failures: [failure],
|
||||||
});
|
});
|
||||||
connectionManager.clearFederatedCall(fedCall.federatedId);
|
|
||||||
}
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user