diff --git a/src/livekit/openIDSFU.test.ts b/src/livekit/openIDSFU.test.ts index 2ddb6c95c..64d7c8389 100644 --- a/src/livekit/openIDSFU.test.ts +++ b/src/livekit/openIDSFU.test.ts @@ -79,8 +79,14 @@ describe("getSFUConfigWithOpenID", () => { ); } catch (ex: unknown) { expect(ex).toBeInstanceOf(FailToGetOpenIdToken); - expect((ex as FailToGetOpenIdToken).cause).toBeInstanceOf(MatrixError); - const mxError = (ex as Error).cause as MatrixError; + // The cause is a wrapper naming the operation we were performing; the + // underlying MatrixError sits behind it. + const context = (ex as FailToGetOpenIdToken).cause as Error; + expect(context.message).toEqual( + "Failed to get a JWT from the legacy endpoint of the MatrixRTC backend at https://sfu.example.org", + ); + expect(context.cause).toBeInstanceOf(MatrixError); + const mxError = context.cause as MatrixError; expect(mxError.message).toEqual( "MatrixError: [500] Failed to look up user info from homeserver", ); @@ -216,8 +222,14 @@ describe("getSFUConfigWithOpenID", () => { ); } catch (ex) { expect(ex).toBeInstanceOf(FailToGetOpenIdToken); - expect((ex as FailToGetOpenIdToken).cause).toBeInstanceOf(MatrixError); - const mxError = (ex as Error).cause as MatrixError; + // The cause is a wrapper naming the operation we were performing; the + // underlying MatrixError sits behind it. + const context = (ex as FailToGetOpenIdToken).cause as Error; + expect(context.message).toEqual( + "Failed to get a JWT from the legacy endpoint of the MatrixRTC backend at https://sfu.example.org", + ); + expect(context.cause).toBeInstanceOf(MatrixError); + const mxError = context.cause as MatrixError; expect(mxError.message).toEqual( "MatrixError: [500] Failed to look up user info from homeserver", ); diff --git a/src/livekit/openIDSFU.ts b/src/livekit/openIDSFU.ts index 00cf69b1a..3dfb045e7 100644 --- a/src/livekit/openIDSFU.ts +++ b/src/livekit/openIDSFU.ts @@ -109,8 +109,14 @@ export async function getSFUConfigWithOpenID( client.getOpenIdToken(), ); } catch (error) { + // Note that in widget mode this is the `get_openid` widget action rather + // than a homeserver request, so the hosting client is a likely culprit. throw new FailToGetOpenIdToken( - error instanceof Error ? error : new Error("Unknown error"), + new Error( + `Failed to get an OpenID token, needed to authenticate with the MatrixRTC backend at ${serviceUrl}` + + `${opts?.delayId ? " (re-authenticating to delegate delayed leave event " + opts.delayId + ")" : ""}`, + { cause: error }, + ), ); } logger?.debug("Got openID token", openIdToken); @@ -142,7 +148,12 @@ export async function getSFUConfigWithOpenID( logger?.debug(`Failed fetching jwt with matrix 2.0 endpoint:`, e); // Make this throw a hard error in case we force the matrix2.0 endpoint. if (forceMatrix2Jwt) { - throw new NoMatrix2AuthorizationService(e as Error); + throw new NoMatrix2AuthorizationService( + new Error( + `Failed to get a JWT from the Matrix 2.0 endpoint of the MatrixRTC backend at ${serviceUrl}`, + { cause: e }, + ), + ); } } } @@ -166,7 +177,10 @@ export async function getSFUConfigWithOpenID( return extractFullConfigFromToken(sfuConfig); } catch (ex) { throw new FailToGetOpenIdToken( - ex instanceof Error ? ex : new Error(`Unknown error ${ex}`), + new Error( + `Failed to get a JWT from the legacy endpoint of the MatrixRTC backend at ${serviceUrl}`, + { cause: ex }, + ), ); } } diff --git a/src/room/GroupCallErrorBoundary.tsx b/src/room/GroupCallErrorBoundary.tsx index 390a5a8c2..ad0836fd5 100644 --- a/src/room/GroupCallErrorBoundary.tsx +++ b/src/room/GroupCallErrorBoundary.tsx @@ -23,10 +23,10 @@ import { } from "@vector-im/compound-design-tokens/assets/web/icons"; import { Button } from "@vector-im/compound-web"; import { logger } from "matrix-js-sdk/lib/logger"; -import { MatrixError } from "matrix-js-sdk"; import { ConnectionLostError, + describeErrorChain, ElementCallError, ErrorCategory, ErrorCode, @@ -80,8 +80,13 @@ const ErrorPage: FC = ({ }); } - const technicalError = - error.cause instanceof MatrixError ? error.cause : null; + // Show the whole cause chain rather than just a `MatrixError` cause: the + // request that actually failed is often several wrappers deep, and errors + // that are not `MatrixError`s (widget API timeouts, LiveKit connection + // errors, ...) used to leave this section empty entirely. A chain of length + // one is just the error we already render above, so keep it hidden. + const errorChain = describeErrorChain(error); + const technicalDetails = errorChain.length > 1 ? errorChain.join("\n") : null; return ( @@ -119,14 +124,12 @@ const ErrorPage: FC = ({ /> )}

- {technicalError ? ( + {technicalDetails ? (
{t("technical_details")} -
-              {technicalError.message}
-            
+
{technicalDetails}
) : null} {actions && @@ -162,7 +165,13 @@ export const GroupCallErrorBoundary = ({ const callError = error instanceof ElementCallError ? error - : new UnknownCallError(error instanceof Error ? error : new Error()); + : new UnknownCallError( + error instanceof Error + ? error + : new Error( + `Non-error value thrown during the call: ${String(error)}`, + ), + ); return ( = ({ ) { setExternalError(new StickyEventsRequiredError()); } else { - setExternalError(new ConnectionLostError()); + logger.error("MembershipManager gave up on the session", error); + // Keep the original error as the cause: the scheduler only tells us + // that it shut down, while the request that actually failed (and its + // errcode) lives further down the cause chain. + setExternalError( + new ConnectionLostError(error instanceof Error ? error : undefined), + ); } }, ); diff --git a/src/state/CallViewModel/localMember/LocalTransport.ts b/src/state/CallViewModel/localMember/LocalTransport.ts index f98a266fd..af2d2f8fb 100644 --- a/src/state/CallViewModel/localMember/LocalTransport.ts +++ b/src/state/CallViewModel/localMember/LocalTransport.ts @@ -179,7 +179,11 @@ export const createLocalTransport$ = ({ `Failed to authenticate to transport ${transport.livekit_service_url}`, e, ); - throw mapAuthErrorToUserFriendlyError(e); + throw mapAuthErrorToUserFriendlyError( + e, + transport.livekit_service_url, + delayId, + ); } }), ); @@ -258,16 +262,26 @@ async function doOpenIdAndJWTFromUrl( }; } -function mapAuthErrorToUserFriendlyError(e: unknown): Error { +function mapAuthErrorToUserFriendlyError( + e: unknown, + serviceUrl: string, + delayId: string | null, +): Error { if ( e instanceof FailToGetOpenIdToken || e instanceof NoMatrix2AuthorizationService ) { - // rethrow as is + // Already carries its own context, rethrow as is. return e; } - // Catch others and rethrow as FailToGetOpenIdToken that has user friendly message. + // Catch others and rethrow as FailToGetOpenIdToken that has user friendly + // message. Record what we were doing, since this branch is reached by + // anything unexpected and otherwise leaves no trace of it. return new FailToGetOpenIdToken( - e instanceof Error ? e : new Error(String(e)), + new Error( + `Unexpected error while authenticating with the MatrixRTC backend at ${serviceUrl}` + + `${delayId ? ` (re-authenticating to delegate delayed leave event ${delayId})` : ""}`, + { cause: e }, + ), ); } diff --git a/src/state/CallViewModel/localMember/RtcTransportAutoDiscovery.ts b/src/state/CallViewModel/localMember/RtcTransportAutoDiscovery.ts index 1f295649f..f118d5e17 100644 --- a/src/state/CallViewModel/localMember/RtcTransportAutoDiscovery.ts +++ b/src/state/CallViewModel/localMember/RtcTransportAutoDiscovery.ts @@ -85,7 +85,10 @@ export class RtcTransportAutoDiscovery { ); } } catch (ex) { - this.logger.info(`Failed to use getRTCTransports end point: ${ex}`); + // Expected against homeservers without MSC4143, so this is not a + // warning, but pass the error itself so the stack and any errcode + // survive into the logs rather than being flattened into a string. + this.logger.info("Failed to use getRTCTransports end point", ex); } return null; } diff --git a/src/state/CallViewModel/remoteMembers/Connection.test.ts b/src/state/CallViewModel/remoteMembers/Connection.test.ts index 723295853..38695b6e8 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.test.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.test.ts @@ -37,6 +37,7 @@ import { import { ObservableScope } from "../../ObservableScope.ts"; import { type OpenIDClientParts } from "../../../livekit/openIDSFU.ts"; import { + describeErrorChain, ElementCallError, FailToGetOpenIdToken, } from "../../../utils/errors.ts"; @@ -284,7 +285,9 @@ describe("Start connection states", () => { capturedState instanceof ElementCallError && capturedState.cause instanceof Error ) { - expect(capturedState.cause.message).toContain( + // The homeserver error is wrapped in a context error naming the + // operation that failed, so assert against the whole cause chain. + expect(describeErrorChain(capturedState).join("\n")).toContain( "Failed to look up user info from homeserver", ); expect(connection.transport.livekit_alias).toEqual( diff --git a/src/utils/errors.ts b/src/utils/errors.ts index 0ac569278..ee17ecaa5 100644 --- a/src/utils/errors.ts +++ b/src/utils/errors.ts @@ -7,6 +7,7 @@ Please see LICENSE in the repository root for full details. import { t } from "i18next"; import { type ConnectionError } from "livekit-client"; +import { HTTPError, MatrixError } from "matrix-js-sdk"; import { i18nKey } from "./i18n"; @@ -95,12 +96,20 @@ export class MatrixRTCTransportMissingError extends ElementCallError { * Error indicating that the connection to the call was lost and could not be re-established. */ export class ConnectionLostError extends ElementCallError { - public constructor() { + /** + * Creates an instance of ConnectionLostError. + * + * @param cause - The underlying error that made us give up on the + * connection, if there is one. It is not shown to the user directly, but it + * ends up in the technical details of the error screen and in Sentry. + */ + public constructor(cause?: Error) { super( t("error.connection_lost"), ErrorCode.CONNECTION_LOST_ERROR, ErrorCategory.NETWORK_CONNECTIVITY, t("error.connection_lost_description"), + cause, ); } } @@ -293,3 +302,76 @@ export class LivekitConnectionError extends ElementCallError { this.localisedMessageValues = { reason: cause.reasonName }; } } + +/** Maximum number of `cause` levels walked by {@link describeErrorChain}. */ +const MAX_CAUSE_DEPTH = 8; + +/** Prefix that `@sentry/react` gives the synthetic error it appends as a cause. */ +const SENTRY_WRAPPER_NAME = "React ErrorBoundary "; + +/** + * Describes a single error, including the protocol level details that Matrix + * and HTTP errors carry but leave out of their `message`. + */ +function describeError(error: unknown): string { + if (error instanceof MatrixError) { + // An `M_UNKNOWN` errcode usually means the failure did not come from the + // homeserver at all: when we run as a widget, the hosting client's widget + // driver serialises any error it cannot map onto a Matrix error that way + // (see `MatrixError.asWidgetApiErrorData`). The url and status are then far + // more telling than the errcode, so always print them. + const parts = [ + `errcode=${error.errcode ?? ""}`, + `httpStatus=${error.httpStatus ?? ""}`, + ]; + if (error.url) parts.push(`url=${error.url}`); + if (error.error) parts.push(`error=${error.error}`); + return `MatrixError(${parts.join(", ")}): ${error.message}`; + } + if (error instanceof HTTPError) + return `HTTPError(httpStatus=${error.httpStatus ?? ""}): ${error.message}`; + if (error instanceof ElementCallError) + return `ElementCallError(code=${error.code}, category=${error.category}): ${error.localisedTitle}`; + if (error instanceof Error) return `${error.name}: ${error.message}`; + return `Non-error value: ${String(error)}`; +} + +/** + * Renders an error together with its whole `cause` chain. + * + * By the time an error reaches the error screen it has usually been wrapped + * several times over (the MembershipManager scheduler, an + * {@link ElementCallError} subclass, a context wrapper at the call site, ...), + * so showing only the outermost error, or a single `cause` hop, tends to hide + * the request that actually failed. + * + * @param error - The error to describe. + * @returns One entry per error in the chain, outermost wrapper first. A chain + * of length one means we have nothing beyond the error itself to report. + */ +export function describeErrorChain(error: unknown): string[] { + const lines: string[] = []; + let current: unknown = error; + for ( + let depth = 0; + current !== undefined && depth < MAX_CAUSE_DEPTH; + depth++ + ) { + // `@sentry/react` appends a synthetic error to the end of the chain to + // carry the React component stack. That is useful in Sentry, but here it + // only repeats the message of the error it is attached to. + if ( + current instanceof Error && + current.name.startsWith(SENTRY_WRAPPER_NAME) + ) + break; + lines.push( + (depth === 0 ? "" : `${" ".repeat(depth)}caused by: `) + + describeError(current), + ); + // Only `Error`s carry a `cause`, and a self-referencing one would loop. + if (!(current instanceof Error) || current.cause === current) break; + current = current.cause; + } + return lines; +}