Skip to content

fix(core): Stop supabaseIntegration from ending spans twice #24126

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
chargome merged 1 commit into from
Sep 8, 2026
+124 −26
Merged

fix(core): Stop supabaseIntegration from ending spans twice #24126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 6 additions & 3 deletions packages/core/src/integrations/supabase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { defineIntegration } from '../integration';
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes';
import { setHttpStatus, SPAN_STATUS_ERROR, SPAN_STATUS_OK } from '../tracing';
import { hasSpanStreamingEnabled } from '../tracing/spans/hasSpanStreamingEnabled';
import { startSpan } from '../tracing/trace';
import { startSpanManual } from '../tracing/trace';
import type { IntegrationFn } from '../types/integration';
import type { WebFetchHeaders } from '../types/webfetchapi';
import { debug } from '../utils/debug-logger';
Expand Down Expand Up @@ -291,7 +291,9 @@ function instrumentAuthOperation(operation: AuthOperationFn, isAdmin = false): A
// transactions.
`auth ${isAdmin ? '(admin) ' : ''}${operation.name}`;

return startSpan(
// The span is ended by hand once the wrapped promise settles, so `startSpanManual` is used to keep
// `startSpan`'s automatic end from ending it a second time.
return startSpanManual(
{
name,
attributes: {
Expand Down Expand Up @@ -474,7 +476,8 @@ function instrumentPostgRESTFilterBuilder(
attributes['db.body'] = bodyPayload;
}

return startSpan(
// Same as the auth wrapper above: the span is ended by hand, so avoid the automatic second end.
return startSpanManual(
{
name,
attributes,
Expand Down
141 changes: 118 additions & 23 deletions packages/core/test/lib/integrations/supabase.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,43 @@ import type {
} from '../../../src/integrations/supabase';
import { resolveDataCollectionOptions } from '../../../src/utils/data-collection/resolveDataCollectionOptions';

const tracingMocks = vi.hoisted(() => ({
startSpan: vi.fn((_opts: unknown, cb: (span: unknown) => unknown) => {
const mockSpan = {
setStatus: vi.fn(),
end: vi.fn(),
};
return cb(mockSpan);
}),
}));
const tracingMocks = vi.hoisted(() => {
const createMockSpan = () => ({
setStatus: vi.fn(),
end: vi.fn(),
});

const startedSpans: Array<ReturnType<typeof createMockSpan>> = [];

return {
startedSpans,
// Mirrors the real `startSpan`, which ends the span itself once the callback's promise settles.
startSpan: vi.fn((_opts: unknown, cb: (span: unknown) => unknown) => {
const mockSpan = createMockSpan();
startedSpans.push(mockSpan);
const result = cb(mockSpan);
if (result && typeof (result as PromiseLike<unknown>).then === 'function') {
return (result as Promise<unknown>).then(
value => {
mockSpan.end();
return value;
},
err => {
mockSpan.end();
throw err;
},
);
}
mockSpan.end();
return result;
}),
startSpanManual: vi.fn((_opts: unknown, cb: (span: unknown, finish: () => void) => unknown) => {
const mockSpan = createMockSpan();
startedSpans.push(mockSpan);
return cb(mockSpan, () => mockSpan.end());

@nicohrubec nicohrubec Sep 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

l: could we test this easily in the integration/e2e level? might be a bit more faithful and we could avoid all this mocking

@chargome chargome Sep 8, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thing is that this is just a no-op in our sdk but in a custom otel setup it emits warnings, so keeping it unit tested is just a regression test in this case.

}),
};
});

const currentScopesMocks = vi.hoisted(() => ({
getClient: vi.fn(),
Expand All @@ -37,6 +65,7 @@ vi.mock('../../../src/tracing', () => ({

vi.mock('../../../src/tracing/trace', () => ({
startSpan: tracingMocks.startSpan,
startSpanManual: tracingMocks.startSpanManual,
}));

vi.mock('../../../src/currentScopes', () => ({
Expand All @@ -53,10 +82,14 @@ type CreateMockSupabaseClientOptions = {
dataCollectionDatabaseQueryData?: boolean;
/** Defaults to `'static'`, so span names keep the full description. */
traceLifecycle?: 'static' | 'stream';
/** When set, the builder's `then` rejects with this value instead of resolving with `resolveWith`. */
rejectWith?: unknown;
};

const DEFAULT_MOCK_SUPABASE_REST_URL = 'https://example.supabase.co/rest/v1/todos';

const flushPromises = (): Promise<void> => new Promise(resolve => setTimeout(resolve, 0));

/** Shared PATCH + query string + body shape for operation data tests. */
const MOCK_SUPABASE_PII_SCENARIO: Pick<CreateMockSupabaseClientOptions, 'method' | 'url' | 'body'> = {
method: 'PATCH',
Expand Down Expand Up @@ -90,7 +123,9 @@ function createMockSupabaseClient(resolveWith: unknown, options?: CreateMockSupa
body = body;

then(onfulfilled?: (value: any) => any, onrejected?: (reason: any) => any): Promise<any> {
return Promise.resolve(resolveWith).then(onfulfilled, onrejected);
const promise =
options?.rejectWith !== undefined ? Promise.reject(options.rejectWith) : Promise.resolve(resolveWith);
return promise.then(onfulfilled, onrejected);
}
}

Expand Down Expand Up @@ -128,6 +163,7 @@ function createMockSupabaseClient(resolveWith: unknown, options?: CreateMockSupa
describe('Supabase Integration', () => {
beforeEach(() => {
currentScopesMocks.getClient.mockReturnValue(undefined);
tracingMocks.startedSpans.length = 0;
});

describe('getHeader', () => {
Expand Down Expand Up @@ -264,6 +300,65 @@ describe('Supabase Integration', () => {
});
});

describe('span lifecycle', () => {
beforeEach(() => {
vi.spyOn(exportsModule, 'captureException').mockImplementation(() => '');
vi.spyOn(breadcrumbModule, 'addBreadcrumb').mockImplementation(() => {});
});

afterEach(() => {
vi.restoreAllMocks();
});

it('ends the PostgREST span exactly once when the query resolves', async () => {
const client = createMockSupabaseClient({ status: 200, data: [] });
instrumentSupabaseClient(client);

await (client as any).from('todos').select('*');
// Awaiting the builder settles through the `resolve` passed to `then`, before the wrapped promise chain
// has fully unwound. Yield once so any trailing `end()` call has had a chance to run.
await flushPromises();

expect(tracingMocks.startedSpans).toHaveLength(1);
expect(tracingMocks.startedSpans[0]!.end).toHaveBeenCalledTimes(1);
});

it('ends the PostgREST span exactly once when the query rejects', async () => {
const rejection = new Error('network down');
const client = createMockSupabaseClient(undefined, { rejectWith: rejection });
instrumentSupabaseClient(client);

await expect((client as any).from('todos').select('*')).rejects.toBe(rejection);
await flushPromises();

expect(tracingMocks.startedSpans).toHaveLength(1);
expect(tracingMocks.startedSpans[0]!.end).toHaveBeenCalledTimes(1);
});

it('ends the auth span exactly once', async () => {
const client = createMockSupabaseClient(undefined) as any;
client.auth.signInWithPassword = vi.fn(() => Promise.resolve({ data: { user: {} }, error: null }));
instrumentSupabaseClient(client);

await client.auth.signInWithPassword({ email: 'a@b.c', password: 'pw' });

expect(tracingMocks.startedSpans).toHaveLength(1);
expect(tracingMocks.startedSpans[0]!.end).toHaveBeenCalledTimes(1);
});

it('ends the auth span exactly once when the operation rejects', async () => {
const client = createMockSupabaseClient(undefined) as any;
const rejection = new Error('auth down');
client.auth.signInWithPassword = vi.fn(() => Promise.reject(rejection));
instrumentSupabaseClient(client);

await expect(client.auth.signInWithPassword({ email: 'a@b.c', password: 'pw' })).rejects.toBe(rejection);

expect(tracingMocks.startedSpans).toHaveLength(1);
expect(tracingMocks.startedSpans[0]!.end).toHaveBeenCalledTimes(1);
});
});

describe('operation data collection', () => {
let captureExceptionSpy: ReturnType<typeof vi.spyOn>;
let addBreadcrumbSpy: ReturnType<typeof vi.spyOn>;
Expand All @@ -286,7 +381,7 @@ describe('Supabase Integration', () => {

await (client as any).from('users').update({}).then();

const spanOptions = tracingMocks.startSpan.mock.calls[0]![0] as {
const spanOptions = tracingMocks.startSpanManual.mock.calls[0]![0] as {
name: string;
attributes: Record<string, unknown>;
};
Expand All @@ -308,7 +403,7 @@ describe('Supabase Integration', () => {

await (client as any).from('users').update({}).then();

const spanOptions = tracingMocks.startSpan.mock.calls[0]![0] as {
const spanOptions = tracingMocks.startSpanManual.mock.calls[0]![0] as {
name: string;
attributes: Record<string, unknown>;
};
Expand Down Expand Up @@ -339,7 +434,7 @@ describe('Supabase Integration', () => {

await (client as any).from('users').update({}).then();

const spanOptions = tracingMocks.startSpan.mock.calls[0]![0] as {
const spanOptions = tracingMocks.startSpanManual.mock.calls[0]![0] as {
name: string;
attributes: Record<string, unknown>;
};
Expand All @@ -361,7 +456,7 @@ describe('Supabase Integration', () => {

await (client as any).from('users').update({}).then();

const spanOptions = tracingMocks.startSpan.mock.calls[0]![0] as {
const spanOptions = tracingMocks.startSpanManual.mock.calls[0]![0] as {
name: string;
attributes: Record<string, unknown>;
};
Expand All @@ -382,7 +477,7 @@ describe('Supabase Integration', () => {

await (client as any).from('users').update({}).then();

const spanOptions = tracingMocks.startSpan.mock.calls[0]![0] as {
const spanOptions = tracingMocks.startSpanManual.mock.calls[0]![0] as {
name: string;
attributes: Record<string, unknown>;
};
Expand All @@ -407,7 +502,7 @@ describe('Supabase Integration', () => {

await (client as any).from('users').update({}).then();

const spanOptions = tracingMocks.startSpan.mock.calls[0]![0] as {
const spanOptions = tracingMocks.startSpanManual.mock.calls[0]![0] as {
name: string;
attributes: Record<string, unknown>;
};
Expand All @@ -432,7 +527,7 @@ describe('Supabase Integration', () => {

await (client as any).from('users').update({}).then();

const spanOptions = tracingMocks.startSpan.mock.calls[0]![0] as {
const spanOptions = tracingMocks.startSpanManual.mock.calls[0]![0] as {
name: string;
attributes: Record<string, unknown>;
};
Expand Down Expand Up @@ -477,7 +572,7 @@ describe('Supabase Integration', () => {
});

it('includes insert(...) in span description and db.body when payload is a non-empty array', async () => {
tracingMocks.startSpan.mockClear();
tracingMocks.startSpanManual.mockClear();
const client = createMockSupabaseClient(
{ status: 200 },
{
Expand All @@ -491,7 +586,7 @@ describe('Supabase Integration', () => {

await (client as any).from('todos').insert({}).then();

const spanOptions = tracingMocks.startSpan.mock.calls[0]![0] as {
const spanOptions = tracingMocks.startSpanManual.mock.calls[0]![0] as {
name: string;
attributes: Record<string, unknown>;
};
Expand All @@ -514,7 +609,7 @@ describe('Supabase Integration', () => {
});

it('sets db.sdk from X-Client-Info', async () => {
tracingMocks.startSpan.mockClear();
tracingMocks.startSpanManual.mockClear();
const client = createMockSupabaseClient(
{ status: 200 },
{ headers: createHeaders({ 'X-Client-Info': 'supabase-js/2.112.0' }) },
Expand All @@ -523,12 +618,12 @@ describe('Supabase Integration', () => {

await (client as any).from('todos').select().then();

const spanOptions = tracingMocks.startSpan.mock.calls[0]![0] as { attributes: Record<string, unknown> };
const spanOptions = tracingMocks.startSpanManual.mock.calls[0]![0] as { attributes: Record<string, unknown> };
expect(spanOptions.attributes['db.sdk']).toBe('supabase-js/2.112.0');
});

it('detects upsert from the Prefer header', async () => {
tracingMocks.startSpan.mockClear();
tracingMocks.startSpanManual.mockClear();
const client = createMockSupabaseClient(
{ status: 200 },
{
Expand All @@ -541,7 +636,7 @@ describe('Supabase Integration', () => {

await (client as any).from('todos').upsert({}).then();

const spanOptions = tracingMocks.startSpan.mock.calls[0]![0] as {
const spanOptions = tracingMocks.startSpanManual.mock.calls[0]![0] as {
name: string;
attributes: Record<string, unknown>;
};
Expand Down
Loading