diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bc271528..129d5370c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Added privacy-scoped setup wizard funnel and Docker startup-failure telemetry with deployment identity handoff, Node.js 20.20.0 support, and cross-platform end-to-end test coverage. [#1653](https://github.com/sourcebot-dev/sourcebot/pull/1653) +### Fixed +- Prevented browser performance instrumentation from breaking code views when `performance.measure()` returns no value. [#1665](https://github.com/sourcebot-dev/sourcebot/pull/1665) + ## [5.1.13] - 2026-09-12 ### Fixed diff --git a/packages/web/src/lib/utils.test.ts b/packages/web/src/lib/utils.test.ts new file mode 100644 index 000000000..1aad40e0a --- /dev/null +++ b/packages/web/src/lib/utils.test.ts @@ -0,0 +1,88 @@ +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { measure, measureSync } from './utils'; + +describe('performance measurement utilities', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + test('measureSync preserves timeline entries without relying on the measure return value', () => { + vi.spyOn(performance, 'now') + .mockReturnValueOnce(100) + .mockReturnValueOnce(112.5); + const markSpy = vi.spyOn(performance, 'mark'); + const measureSpy = vi.spyOn(performance, 'measure') + .mockImplementation(() => undefined as unknown as PerformanceMeasure); + + const result = measureSync(() => 'result', 'sync-operation', false); + + expect(result).toEqual({ + data: 'result', + durationMs: 12.5, + }); + expect(markSpy).toHaveBeenNthCalledWith(1, 'sync-operation.start'); + expect(markSpy).toHaveBeenNthCalledWith(2, 'sync-operation.end'); + expect(measureSpy).toHaveBeenCalledWith( + 'sync-operation', + 'sync-operation.start', + 'sync-operation.end', + ); + }); + + test('measure preserves timeline entries without relying on the measure return value', async () => { + vi.spyOn(performance, 'now') + .mockReturnValueOnce(50) + .mockReturnValueOnce(75); + const markSpy = vi.spyOn(performance, 'mark'); + const measureSpy = vi.spyOn(performance, 'measure') + .mockImplementation(() => undefined as unknown as PerformanceMeasure); + + const result = await measure(async () => 'result', 'async-operation', false); + + expect(result).toEqual({ + data: 'result', + durationMs: 25, + }); + expect(markSpy).toHaveBeenNthCalledWith(1, 'async-operation.start'); + expect(markSpy).toHaveBeenNthCalledWith(2, 'async-operation.end'); + expect(measureSpy).toHaveBeenCalledWith( + 'async-operation', + 'async-operation.start', + 'async-operation.end', + ); + }); + + test('ignores performance timeline instrumentation failures', () => { + vi.spyOn(performance, 'now') + .mockReturnValueOnce(10) + .mockReturnValueOnce(15); + vi.spyOn(performance, 'mark').mockImplementation(() => { + throw new Error('Performance timeline unavailable'); + }); + vi.spyOn(performance, 'measure').mockImplementation(() => { + throw new Error('Performance timeline unavailable'); + }); + + expect(measureSync(() => 'result', 'operation', false)).toEqual({ + data: 'result', + durationMs: 5, + }); + }); + + test('measure ignores performance timeline instrumentation failures', async () => { + vi.spyOn(performance, 'now') + .mockReturnValueOnce(20) + .mockReturnValueOnce(27.5); + vi.spyOn(performance, 'mark').mockImplementation(() => { + throw new Error('Performance timeline unavailable'); + }); + vi.spyOn(performance, 'measure').mockImplementation(() => { + throw new Error('Performance timeline unavailable'); + }); + + await expect(measure(async () => 'async result', 'async-operation', false)).resolves.toEqual({ + data: 'async result', + durationMs: 7.5, + }); + }); +}); diff --git a/packages/web/src/lib/utils.ts b/packages/web/src/lib/utils.ts index b49050f2a..d51e217a3 100644 --- a/packages/web/src/lib/utils.ts +++ b/packages/web/src/lib/utils.ts @@ -543,16 +543,34 @@ export const formatCurrency = ( return formatter.format(amountSmallestUnit / Math.pow(10, fractionDigits)); } +const recordPerformanceMark = (markName: string) => { + try { + performance.mark(markName); + } catch { + // Performance timeline instrumentation must not break the measured operation. + } +} + +const recordPerformanceMeasure = (measureName: string, startMark: string, endMark: string) => { + try { + performance.measure(measureName, startMark, endMark); + } catch { + // Performance timeline instrumentation must not break the measured operation. + } +} + export const measureSync = (cb: () => T, measureName: string, outputLog: boolean = true) => { const startMark = `${measureName}.start`; const endMark = `${measureName}.end`; - performance.mark(startMark); + recordPerformanceMark(startMark); + const startTime = performance.now(); const data = cb(); - performance.mark(endMark); + const endTime = performance.now(); + recordPerformanceMark(endMark); - const measure = performance.measure(measureName, startMark, endMark); - const durationMs = measure.duration; + recordPerformanceMeasure(measureName, startMark, endMark); + const durationMs = endTime - startTime; if (outputLog) { console.debug(`[${measureName}] took ${durationMs}ms`); } @@ -567,12 +585,14 @@ export const measure = async (cb: () => Promise, measureName: string, outp const startMark = `${measureName}.start`; const endMark = `${measureName}.end`; - performance.mark(startMark); + recordPerformanceMark(startMark); + const startTime = performance.now(); const data = await cb(); - performance.mark(endMark); + const endTime = performance.now(); + recordPerformanceMark(endMark); - const measure = performance.measure(measureName, startMark, endMark); - const durationMs = measure.duration; + recordPerformanceMeasure(measureName, startMark, endMark); + const durationMs = endTime - startTime; if (outputLog) { console.debug(`[${measureName}] took ${durationMs}ms`); }