Skip to content

Commit d8fff32

Browse files
pierre-lehnen-rcricardogarim
pierre-lehnen-rc
authored and
ricardogarim
committed
regression: call ending on non-critical errors (#37070)
1 parent e74385e commit d8fff32

File tree

9 files changed

+146
-50
lines changed

9 files changed

+146
-50
lines changed

‎ee/packages/media-calls/src/internal/agents/CallSignalProcessor.ts‎

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ export class UserActorSignalProcessor {
9696
case 'local-state':
9797
return this.reviewLocalState(signal);
9898
case 'error':
99-
return this.processError(signal.errorType, signal.errorCode);
99+
return this.processError(signal);
100100
case 'negotiation-needed':
101101
return this.processNegotiationNeeded(signal.oldNegotiationId);
102102
case 'transfer':
@@ -135,19 +135,44 @@ export class UserActorSignalProcessor {
135135
}
136136
}
137137

138-
private async processError(errorType: ClientMediaSignalError['errorType'], errorCode?: string): Promise<void> {
138+
private async processError(signal: ClientMediaSignalError): Promise<void> {
139139
if (!this.signed) {
140140
return;
141141
}
142142

143-
switch (errorType) {
144-
case 'signaling':
145-
return this.onSignalingError(errorCode);
146-
case 'service':
147-
return this.onServiceError(errorCode);
148-
default:
149-
return this.onUnexpectedError(errorCode);
143+
const { errorType = 'other', errorCode, critical = false, negotiationId, errorDetails } = signal;
144+
logger.error({
145+
msg: 'Client reported an error',
146+
errorType,
147+
errorCode,
148+
critical,
149+
errorDetails,
150+
negotiationId,
151+
callId: this.callId,
152+
role: this.role,
153+
state: this.call.state,
154+
});
155+
156+
let hangupReason: CallHangupReason = 'error';
157+
if (errorType === 'service') {
158+
hangupReason = 'service-error';
159+
160+
// Do not hangup on service errors after the call is already active;
161+
// if the error happened on a renegotiation, then the service may still be able to rollback to a valid state
162+
if (this.isPastNegotiation()) {
163+
return;
164+
}
165+
}
166+
167+
if (!critical) {
168+
return;
150169
}
170+
171+
if (errorType === 'signaling') {
172+
hangupReason = 'signaling-error';
173+
}
174+
175+
await mediaCallDirector.hangup(this.call, this.agent, hangupReason);
151176
}
152177

153178
private async processNegotiationNeeded(oldNegotiationId: string): Promise<void> {
@@ -273,29 +298,4 @@ export class UserActorSignalProcessor {
273298
await this.clientIsActive();
274299
}
275300
}
276-
277-
private async onSignalingError(errorMessage?: string): Promise<void> {
278-
logger.error({ msg: 'Client reported a signaling error', errorMessage, callId: this.callId, role: this.role, state: this.call.state });
279-
await mediaCallDirector.hangup(this.call, this.agent, 'signaling-error');
280-
}
281-
282-
private async onServiceError(errorMessage?: string): Promise<void> {
283-
logger.error({ msg: 'Client reported a service error', errorMessage, callId: this.callId, role: this.role, state: this.call.state });
284-
if (this.isPastNegotiation()) {
285-
return;
286-
}
287-
288-
await mediaCallDirector.hangup(this.call, this.agent, 'service-error');
289-
}
290-
291-
private async onUnexpectedError(errorMessage?: string): Promise<void> {
292-
logger.error({
293-
msg: 'Client reported an unexpected error',
294-
errorMessage,
295-
callId: this.callId,
296-
role: this.role,
297-
state: this.call.state,
298-
});
299-
await mediaCallDirector.hangup(this.call, this.agent, 'error');
300-
}
301301
}

‎packages/media-signaling/src/definition/call/IClientMediaCall.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ export type CallHangupReason =
3636
| 'signaling-error' // Hanging up because of an error during the signal processing
3737
| 'service-error' // Hanging up because of an error setting up the service connection
3838
| 'media-error' // Hanging up because of an error setting up the media connection
39+
| 'input-error' // Something wrong with the audio input track on the client
3940
| 'error' // Hanging up because of an unidentified error
4041
| 'unknown'; // One of the call's signed users reported they don't know this call
4142

‎packages/media-signaling/src/definition/services/IServiceProcessor.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export type ServiceStateValue<ServiceStateMap extends DefaultServiceStateMap, K
1313

1414
export type ServiceProcessorEvents<ServiceStateMap extends DefaultServiceStateMap> = {
1515
internalStateChange: keyof ServiceStateMap;
16-
internalError: { critical: boolean; error: string | Error };
16+
internalError: { critical: boolean; error: string | Error; errorDetails?: string };
1717
negotiationNeeded: void;
1818
};
1919

‎packages/media-signaling/src/definition/signals/client/error.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ export type ClientMediaSignalError = {
99
errorType?: 'signaling' | 'service' | 'other';
1010
errorCode?: string;
1111
negotiationId?: string;
12+
critical?: boolean;
13+
errorDetails?: string;
1214
};
1315

1416
export const clientMediaSignalErrorSchema: JSONSchemaType<ClientMediaSignalError> = {
@@ -41,6 +43,14 @@ export const clientMediaSignalErrorSchema: JSONSchemaType<ClientMediaSignalError
4143
type: 'string',
4244
nullable: true,
4345
},
46+
critical: {
47+
type: 'boolean',
48+
nullable: true,
49+
},
50+
errorDetails: {
51+
type: 'string',
52+
nullable: true,
53+
},
4454
},
4555
additionalProperties: false,
4656
required: ['callId', 'contractId', 'type'],

‎packages/media-signaling/src/lib/Call.ts‎

Lines changed: 57 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import type { ClientContractState, ClientState } from '../definition/client';
1616
import type { IMediaSignalLogger } from '../definition/logger';
1717
import type { IWebRTCProcessor, WebRTCInternalStateMap } from '../definition/services';
1818
import { isPendingState } from './services/states';
19+
import { serializeError } from './utils/serializeError';
1920
import type {
2021
ServerMediaSignal,
2122
ServerMediaSignalNewCall,
@@ -275,6 +276,12 @@ export class ClientMediaCall implements IClientMediaCall {
275276
try {
276277
this.prepareWebRtcProcessor();
277278
} catch (e) {
279+
this.sendError({
280+
errorType: 'service',
281+
errorCode: 'service-initialization-failed',
282+
critical: true,
283+
errorDetails: serializeError(e),
284+
});
278285
await this.rejectAsUnavailable();
279286
throw e;
280287
}
@@ -725,7 +732,7 @@ export class ClientMediaCall implements IClientMediaCall {
725732
const { negotiationId } = signal;
726733

727734
if (this.shouldIgnoreWebRTC()) {
728-
this.sendError({ errorType: 'service', errorCode: 'invalid-service', negotiationId });
735+
this.sendError({ errorType: 'service', errorCode: 'invalid-service', negotiationId, critical: true });
729736
return;
730737
}
731738

@@ -742,12 +749,19 @@ export class ClientMediaCall implements IClientMediaCall {
742749
try {
743750
offer = await this.webrtcProcessor.createOffer({ iceRestart });
744751
} catch (e) {
745-
this.sendError({ errorType: 'service', errorCode: 'failed-to-create-offer', negotiationId });
752+
this.sendError({
753+
errorType: 'service',
754+
errorCode: 'failed-to-create-offer',
755+
negotiationId,
756+
critical: true,
757+
errorDetails: serializeError(e),
758+
});
746759
throw e;
747760
}
748761

749762
if (!offer) {
750-
this.sendError({ errorType: 'service', errorCode: 'implementation-error', negotiationId });
763+
this.sendError({ errorType: 'service', errorCode: 'implementation-error', negotiationId, critical: true });
764+
return;
751765
}
752766

753767
await this.deliverSdp({ ...offer, negotiationId });
@@ -797,12 +811,18 @@ export class ClientMediaCall implements IClientMediaCall {
797811
answer = await this.webrtcProcessor.createAnswer(signal);
798812
} catch (e) {
799813
this.config.logger?.error(e);
800-
this.sendError({ errorType: 'service', errorCode: 'failed-to-create-answer', negotiationId });
814+
this.sendError({
815+
errorType: 'service',
816+
errorCode: 'failed-to-create-answer',
817+
negotiationId,
818+
critical: true,
819+
errorDetails: serializeError(e),
820+
});
801821
throw e;
802822
}
803823

804824
if (!answer) {
805-
this.sendError({ errorType: 'service', errorCode: 'implementation-error', negotiationId });
825+
this.sendError({ errorType: 'service', errorCode: 'implementation-error', negotiationId, critical: true });
806826
return;
807827
}
808828

@@ -930,7 +950,7 @@ export class ClientMediaCall implements IClientMediaCall {
930950
}
931951

932952
if (!this.acceptedLocally) {
933-
this.config.transporter.sendError(this.callId, { errorType: 'signaling', errorCode: 'not-accepted' });
953+
this.config.transporter.sendError(this.callId, { errorType: 'signaling', errorCode: 'not-accepted', critical: true });
934954
this.config.logger?.error('Trying to activate a call that was not yet accepted locally.');
935955
return;
936956
}
@@ -1033,14 +1053,25 @@ export class ClientMediaCall implements IClientMediaCall {
10331053
}
10341054
}
10351055

1036-
private onWebRTCInternalError({ critical, error }: { critical: boolean; error: string | Error }): void {
1056+
private onWebRTCInternalError({
1057+
critical,
1058+
error,
1059+
errorDetails,
1060+
}: {
1061+
critical: boolean;
1062+
error: string | Error;
1063+
errorDetails?: string;
1064+
}): void {
10371065
this.config.logger?.debug('ClientMediaCall.onWebRTCInternalError', critical, error);
10381066
const errorCode = typeof error === 'object' ? error.message : error;
1039-
this.sendError({ errorType: 'service', errorCode, ...(this.currentNegotiationId && { negotiationId: this.currentNegotiationId }) });
10401067

1041-
if (critical) {
1042-
this.hangup('service-error');
1043-
}
1068+
this.sendError({
1069+
errorType: 'service',
1070+
errorCode,
1071+
...(this.currentNegotiationId && { negotiationId: this.currentNegotiationId }),
1072+
...(errorDetails && { errorDetails }),
1073+
critical,
1074+
});
10441075
}
10451076

10461077
private onWebRTCNegotiationNeeded(): void {
@@ -1069,11 +1100,25 @@ export class ClientMediaCall implements IClientMediaCall {
10691100
break;
10701101
case 'failed':
10711102
if (!this.isOver()) {
1103+
this.sendError({
1104+
errorType: 'service',
1105+
errorCode: 'connection-failed',
1106+
critical: true,
1107+
negotiationId: this.currentNegotiationId || undefined,
1108+
});
1109+
10721110
this.hangup('service-error');
10731111
}
10741112
break;
10751113
case 'closed':
10761114
if (!this.isOver()) {
1115+
this.sendError({
1116+
errorType: 'service',
1117+
errorCode: 'connection-closed',
1118+
critical: true,
1119+
negotiationId: this.currentNegotiationId || undefined,
1120+
});
1121+
10771122
this.hangup('service-error');
10781123
}
10791124
break;
@@ -1143,7 +1188,7 @@ export class ClientMediaCall implements IClientMediaCall {
11431188
try {
11441189
this.prepareWebRtcProcessor();
11451190
} catch (e) {
1146-
this.sendError({ errorType: 'service', errorCode: 'webrtc-not-implemented' });
1191+
this.sendError({ errorType: 'service', errorCode: 'webrtc-not-implemented', critical: true, errorDetails: serializeError(e) });
11471192
throw e;
11481193
}
11491194
}

‎packages/media-signaling/src/lib/Session.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ export class MediaSignalingSession extends Emitter<MediaSignalingEvents> {
426426
}
427427

428428
try {
429-
call.hangup('service-error');
429+
call.hangup('input-error');
430430
} catch {
431431
//
432432
}

‎packages/media-signaling/src/lib/TransportWrapper.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,13 @@ export class MediaSignalTransportWrapper {
2626
} as GenericClientMediaSignal<T>);
2727
}
2828

29-
public sendError(callId: string, { errorType, errorCode, negotiationId }: Partial<ClientMediaSignalError>) {
29+
public sendError(callId: string, { errorType, errorCode, negotiationId, critical, errorDetails }: Partial<ClientMediaSignalError>) {
3030
this.sendToServer(callId, 'error', {
3131
errorType: errorType || 'other',
3232
...(errorCode && { errorCode }),
3333
...(negotiationId && { negotiationId }),
34+
...(critical ? { critical } : { critical: false }),
35+
...(errorDetails && { errorDetails }),
3436
});
3537
}
3638

‎packages/media-signaling/src/lib/services/webrtc/Processor.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,8 @@ export class MediaCallWebRTCProcessor implements IWebRTCProcessor {
325325
}
326326
this.config.logger?.debug('MediaCallWebRTCProcessor.onIceCandidateError');
327327
this.config.logger?.error(event);
328-
this.emitter.emit('internalError', { critical: false, error: 'ice-candidate-error' });
328+
329+
this.emitter.emit('internalError', { critical: false, error: 'ice-candidate-error', errorDetails: JSON.stringify(event) });
329330
}
330331

331332
private onNegotiationNeeded() {

‎packages/media-signaling/src/lib/utils/serializeError.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
export function serializeError(error: unknown): string | undefined {
2+
try {
3+
if (!error) {
4+
return undefined;
5+
}
6+
7+
if (typeof error === 'string') {
8+
return error;
9+
}
10+
11+
if (typeof error === 'object') {
12+
if (error instanceof Error) {
13+
return JSON.stringify({
14+
...error,
15+
name: error.name,
16+
message: error.message,
17+
});
18+
}
19+
20+
const errorData: Record<string, any> = { ...error };
21+
if ('name' in error) {
22+
errorData.name = error.name;
23+
}
24+
if ('message' in error) {
25+
errorData.message = error.message;
26+
}
27+
28+
if (Object.keys(errorData).length > 0) {
29+
return JSON.stringify(errorData);
30+
}
31+
}
32+
} catch {
33+
//
34+
}
35+
36+
return undefined;
37+
}

0 commit comments

Comments
 (0)