Skip to content

Commit 119c06f

Browse files
antonisclaude
andauthored
fix(core): Guard nullish response in supabase PostgREST handler (#20033)
Closes #20032 ### Context: In the `supabaseIntegration`'s PostgREST instrumentation, the `.then()` success handler accesses `res.error` without checking if `res` is nullish first. This causes crashes in environments like React Native where the response can be `undefined`. A related error recently trended on the React Native SDK (see Linear comment) ### Summary: - Added a null guard on `res` before accessing `res.error` in `instrumentPostgRESTFilterBuilder`, changing `if (res.error)` to `if (res && res.error)` — matching the existing pattern used in `instrumentAuthOperation` - The existing `setHttpStatus` block already had a proper guard (`if (res && typeof res === 'object' && 'status' in res)`), so only the error-handling path was affected - Span `.end()` and breadcrumb creation continue to work correctly regardless of whether `res` is nullish - Added a new test file for the supabase integration covering the nullish response scenario and existing utility functions Before submitting a pull request, please take a look at our [Contributing](https://github.com/getsentry/sentry-javascript/blob/master/CONTRIBUTING.md) guidelines and verify: - [x] If you've added code that should be tested, please add tests. - [x] Ensure your code lints and the test suite passes (`yarn lint`) & (`yarn test`). - [x] Link an issue if there is one related to your pull request. If no issue is linked, one will be auto-generated and linked. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent fff880e commit 119c06f

File tree

2 files changed

+180
-1
lines changed

2 files changed

+180
-1
lines changed

packages/core/src/integrations/supabase.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ function instrumentPostgRESTFilterBuilder(PostgRESTFilterBuilder: PostgRESTFilte
403403
span.end();
404404
}
405405

406-
if (res.error) {
406+
if (res?.error) {
407407
const err = new Error(res.error.message) as SupabaseError;
408408
if (res.error.code) {
409409
err.code = res.error.code;
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
2+
import * as breadcrumbModule from '../../../src/breadcrumbs';
3+
import * as exportsModule from '../../../src/exports';
4+
import {
5+
extractOperation,
6+
instrumentSupabaseClient,
7+
translateFiltersIntoMethods,
8+
} from '../../../src/integrations/supabase';
9+
import type { PostgRESTQueryBuilder, SupabaseClientInstance } from '../../../src/integrations/supabase';
10+
11+
// Mock tracing to avoid needing full SDK setup
12+
vi.mock('../../../src/tracing', () => ({
13+
startSpan: (_opts: any, cb: (span: any) => any) => {
14+
const mockSpan = {
15+
setStatus: vi.fn(),
16+
end: vi.fn(),
17+
};
18+
return cb(mockSpan);
19+
},
20+
setHttpStatus: vi.fn(),
21+
SPAN_STATUS_OK: 1,
22+
SPAN_STATUS_ERROR: 2,
23+
}));
24+
25+
describe('Supabase Integration', () => {
26+
describe('extractOperation', () => {
27+
it('returns select for GET', () => {
28+
expect(extractOperation('GET')).toBe('select');
29+
});
30+
31+
it('returns insert for POST without resolution header', () => {
32+
expect(extractOperation('POST')).toBe('insert');
33+
});
34+
35+
it('returns upsert for POST with resolution header', () => {
36+
expect(extractOperation('POST', { Prefer: 'resolution=merge-duplicates' })).toBe('upsert');
37+
});
38+
39+
it('returns update for PATCH', () => {
40+
expect(extractOperation('PATCH')).toBe('update');
41+
});
42+
43+
it('returns delete for DELETE', () => {
44+
expect(extractOperation('DELETE')).toBe('delete');
45+
});
46+
});
47+
48+
describe('translateFiltersIntoMethods', () => {
49+
it('returns select(*) for wildcard', () => {
50+
expect(translateFiltersIntoMethods('select', '*')).toBe('select(*)');
51+
});
52+
53+
it('returns select with columns', () => {
54+
expect(translateFiltersIntoMethods('select', 'id,name')).toBe('select(id,name)');
55+
});
56+
57+
it('translates eq filter', () => {
58+
expect(translateFiltersIntoMethods('id', 'eq.123')).toBe('eq(id, 123)');
59+
});
60+
});
61+
62+
describe('instrumentPostgRESTFilterBuilder - nullish response handling', () => {
63+
let captureExceptionSpy: ReturnType<typeof vi.spyOn>;
64+
let addBreadcrumbSpy: ReturnType<typeof vi.spyOn>;
65+
66+
beforeEach(() => {
67+
captureExceptionSpy = vi.spyOn(exportsModule, 'captureException').mockImplementation(() => '');
68+
addBreadcrumbSpy = vi.spyOn(breadcrumbModule, 'addBreadcrumb').mockImplementation(() => {});
69+
});
70+
71+
afterEach(() => {
72+
vi.restoreAllMocks();
73+
});
74+
75+
function createMockSupabaseClient(resolveWith: unknown): unknown {
76+
// Create a PostgRESTFilterBuilder-like class
77+
class MockPostgRESTFilterBuilder {
78+
method = 'GET';
79+
headers: Record<string, string> = { 'X-Client-Info': 'supabase-js/2.0.0' };
80+
url = new URL('https://example.supabase.co/rest/v1/todos');
81+
schema = 'public';
82+
body = undefined;
83+
84+
then(onfulfilled?: (value: any) => any, onrejected?: (reason: any) => any): Promise<any> {
85+
return Promise.resolve(resolveWith).then(onfulfilled, onrejected);
86+
}
87+
}
88+
89+
class MockPostgRESTQueryBuilder {
90+
select() {
91+
return new MockPostgRESTFilterBuilder();
92+
}
93+
insert() {
94+
return new MockPostgRESTFilterBuilder();
95+
}
96+
upsert() {
97+
return new MockPostgRESTFilterBuilder();
98+
}
99+
update() {
100+
return new MockPostgRESTFilterBuilder();
101+
}
102+
delete() {
103+
return new MockPostgRESTFilterBuilder();
104+
}
105+
}
106+
107+
// Create a mock SupabaseClient constructor
108+
class MockSupabaseClient {
109+
auth = {
110+
admin: {} as any,
111+
} as SupabaseClientInstance['auth'];
112+
113+
from(_table: string): PostgRESTQueryBuilder {
114+
return new MockPostgRESTQueryBuilder() as unknown as PostgRESTQueryBuilder;
115+
}
116+
}
117+
118+
return new MockSupabaseClient();
119+
}
120+
121+
it('handles undefined response without throwing', async () => {
122+
const client = createMockSupabaseClient(undefined);
123+
instrumentSupabaseClient(client);
124+
125+
const builder = (client as any).from('todos');
126+
const result = builder.select('*');
127+
128+
// This should not throw even though the response is undefined
129+
const res = await result;
130+
expect(res).toBeUndefined();
131+
});
132+
133+
it('handles null response without throwing', async () => {
134+
const client = createMockSupabaseClient(null);
135+
instrumentSupabaseClient(client);
136+
137+
const builder = (client as any).from('todos');
138+
const result = builder.select('*');
139+
140+
const res = await result;
141+
expect(res).toBeNull();
142+
});
143+
144+
it('still adds breadcrumb when response is undefined', async () => {
145+
const client = createMockSupabaseClient(undefined);
146+
instrumentSupabaseClient(client);
147+
148+
const builder = (client as any).from('todos');
149+
await builder.select('*');
150+
151+
expect(addBreadcrumbSpy).toHaveBeenCalledWith(
152+
expect.objectContaining({
153+
type: 'supabase',
154+
category: 'db.select',
155+
}),
156+
);
157+
});
158+
159+
it('does not capture exception when response is undefined', async () => {
160+
const client = createMockSupabaseClient(undefined);
161+
instrumentSupabaseClient(client);
162+
163+
const builder = (client as any).from('todos');
164+
await builder.select('*');
165+
166+
expect(captureExceptionSpy).not.toHaveBeenCalled();
167+
});
168+
169+
it('still captures error when response has error', async () => {
170+
const client = createMockSupabaseClient({ status: 400, error: { message: 'Bad request', code: '400' } });
171+
instrumentSupabaseClient(client);
172+
173+
const builder = (client as any).from('todos');
174+
await builder.select('*');
175+
176+
expect(captureExceptionSpy).toHaveBeenCalled();
177+
});
178+
});
179+
});

0 commit comments

Comments
 (0)