From 49397486b2fd94c926e41064f6500b5d5d25601a Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Tue, 1 Sep 2026 22:02:37 +0200 Subject: [PATCH] fix(crypto): send verification diagnostics to Sentry instead of the opt-in debug logger --- src/app/crypto/engineCrypto/EngineCrypto.ts | 13 ++++--- src/app/utils/verificationTrace.test.ts | 40 +++++++++++++++++++++ src/app/utils/verificationTrace.ts | 33 +++++++++++++++++ 3 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 src/app/utils/verificationTrace.test.ts create mode 100644 src/app/utils/verificationTrace.ts diff --git a/src/app/crypto/engineCrypto/EngineCrypto.ts b/src/app/crypto/engineCrypto/EngineCrypto.ts index 7534cea48e..f49237c4d7 100644 --- a/src/app/crypto/engineCrypto/EngineCrypto.ts +++ b/src/app/crypto/engineCrypto/EngineCrypto.ts @@ -37,6 +37,7 @@ import { secretStorageCanAccessSecrets } from './secretStorageAccess'; import { PerSessionBackupDownloader } from './perSessionBackupDownload'; import type { CryptoEventHandlerMap } from 'matrix-js-sdk/lib/crypto-api/CryptoEventHandlerMap'; import { createDebugLogger } from '$utils/debugLogger'; +import { traceVerification, warnVerification } from '$utils/verificationTrace'; import { EngineVerificationRequest } from '../verification/request'; import { EnginePhase, @@ -738,7 +739,7 @@ export class EngineCrypto } const request = new EngineVerificationRequest(this.#engineCall, state); this.#verificationRequests.set(transactionId, request); - engineCryptoLog.info('general', 'Surfacing an incoming verification request', { + traceVerification('Surfacing an incoming verification request', { sender, transactionId, isSelfVerification: state.isSelfVerification, @@ -759,7 +760,7 @@ export class EngineCrypto await this.#receiveSyncChanges({ toDeviceEvents: [event] }); if (!(await this.onIncomingKeyVerificationRequest(sender, transactionId))) { const sentAt = (event.content as { timestamp?: number } | undefined)?.timestamp; - engineCryptoLog.warn('general', 'The engine kept ignoring a verification request', { + warnVerification('The engine kept ignoring a verification request', { sender, transactionId, clockSkewSeconds: @@ -835,7 +836,7 @@ export class EngineCrypto if (typeof message.type === 'string' && message.type.startsWith('m.key.verification.')) { const transactionId = (message.content as { transaction_id?: string })?.transaction_id; if (transactionId && message.sender) { - engineCryptoLog.info('general', 'Received a verification to-device event', { + traceVerification('Received a verification to-device event', { type: message.type, sender: message.sender, transactionId, @@ -943,7 +944,9 @@ export class EngineCrypto userId: this.#identity.userId, })) ?? []) as EngineVerificationState[]; } catch (error) { - engineCryptoLog.warn('general', 'Could not list pending verification requests', error); + warnVerification('Could not list pending verification requests', { + reason: error instanceof Error ? error.message : String(error), + }); return; } @@ -953,7 +956,7 @@ export class EngineCrypto const request = new EngineVerificationRequest(this.#engineCall, state); this.#verificationRequests.set(state.flowId, request); - engineCryptoLog.warn('general', 'Recovered a verification request the sync path missed', { + warnVerification('Recovered a verification request the sync path missed', { flowId: state.flowId, otherUserId: state.otherUserId, phase: state.phase, diff --git a/src/app/utils/verificationTrace.test.ts b/src/app/utils/verificationTrace.test.ts new file mode 100644 index 0000000000..05fd3619a2 --- /dev/null +++ b/src/app/utils/verificationTrace.test.ts @@ -0,0 +1,40 @@ +import * as Sentry from '@sentry/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { traceVerification, warnVerification } from './verificationTrace'; + +vi.mock('@sentry/react', () => ({ + addBreadcrumb: vi.fn<(input: unknown) => void>(), + logger: { + info: vi.fn<(message: string, attrs: unknown) => void>(), + warn: vi.fn<(message: string, attrs: unknown) => void>(), + }, +})); + +describe('verification tracing', () => { + beforeEach(() => vi.clearAllMocks()); + + it('reaches Sentry without depending on the opt-in debug logger', () => { + traceVerification('Received a verification to-device event', { + sender: '@me:e.org', + transactionId: '$f', + }); + + expect(Sentry.logger.info).toHaveBeenCalledWith( + '[crypto:verification] Received a verification to-device event', + { sender: '@me:e.org', transactionId: '$f' } + ); + expect(Sentry.addBreadcrumb).toHaveBeenCalledOnce(); + }); + + it('drops attributes Sentry cannot index instead of failing', () => { + warnVerification('The engine kept ignoring a verification request', { + sender: '@me:e.org', + clockSkewSeconds: null, + }); + + expect(Sentry.logger.warn).toHaveBeenCalledWith( + '[crypto:verification] The engine kept ignoring a verification request', + { sender: '@me:e.org' } + ); + }); +}); diff --git a/src/app/utils/verificationTrace.ts b/src/app/utils/verificationTrace.ts new file mode 100644 index 0000000000..3a881330f2 --- /dev/null +++ b/src/app/utils/verificationTrace.ts @@ -0,0 +1,33 @@ +import * as Sentry from '@sentry/react'; + +type TraceData = Record; + +const attributes = (data: TraceData): Record => { + const out: Record = {}; + Object.entries(data).forEach(([key, value]) => { + if (typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean') { + out[key] = value; + } + }); + return out; +}; + +export const traceVerification = (message: string, data: TraceData = {}): void => { + Sentry.addBreadcrumb({ + category: 'crypto.verification', + message, + level: 'info', + data: attributes(data), + }); + Sentry.logger.info(`[crypto:verification] ${message}`, attributes(data)); +}; + +export const warnVerification = (message: string, data: TraceData = {}): void => { + Sentry.addBreadcrumb({ + category: 'crypto.verification', + message, + level: 'warning', + data: attributes(data), + }); + Sentry.logger.warn(`[crypto:verification] ${message}`, attributes(data)); +};