Skip to content

fix(web): make performance timing non-blocking #1665

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jsourcebot merged 3 commits into from
Sep 17, 2026
Merged

fix(web): make performance timing non-blocking #1665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
45429eb
fix(web): make performance timing non-blocking
jsourcebot Sep 17, 2026
a970852
docs: add changelog entry for #1665
jsourcebot Sep 17, 2026
68d7cd2
test(web): cover async timing instrumentation failures
jsourcebot Sep 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
88 changes: 88 additions & 0 deletions packages/web/src/lib/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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', () => {

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Add an async regression test where performance.mark() and performance.measure() throw, and assert that measure() still returns the callback result with the timestamp-derived duration.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At packages/web/src/lib/utils.test.ts, line 55:

<comment>Add an async regression test where `performance.mark()` and `performance.measure()` throw, and assert that `measure()` still returns the callback result with the timestamp-derived duration.</comment>

<file context>
@@ -0,0 +1,71 @@
+        );
+    });
+
+    test('ignores performance timeline instrumentation failures', () => {
+        vi.spyOn(performance, 'now')
+            .mockReturnValueOnce(10)
</file context>

Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
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,
});
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

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,
});
});
});
36 changes: 28 additions & 8 deletions packages/web/src/lib/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <T>(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`);
}
Expand All @@ -567,12 +585,14 @@ export const measure = async <T>(cb: () => Promise<T>, 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`);
}
Expand Down
Loading