Skip to content

Commit 3b3d649

Browse files
yosiharanclaude
andauthored
fix(fga): pre-consume cache response body to prevent node-fetch clone hang when using fgaCacheUrl (#700)
* fix(fga): pre-consume cache response body to prevent node-fetch clone hang When fgaCacheUrl is set, postWithOptionalCache calls fetch() directly, bypassing fetchWrapper. The raw node-fetch Response was returned to transformResponse(), which calls clone().json() — the exact pattern fetchWrapper avoids with an explicit workaround for a known node-fetch issue where cloning a consumed body stream can hang indefinitely. Apply the same body pre-consumption fix (read text once, override clone/text/json with memoized versions) to postWithOptionalCache in both fga.ts and authz.ts. Fixes #699 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(fga): address PR review comments - Make json() override async so JSON.parse errors become rejected Promises rather than synchronous throws, matching Response.json() contract - Use mockImplementation for 1000-call concurrent test to return a fresh response object per call and avoid shared-mutation issues - Update regression test comment to remove stale "Skipped" wording and clarify the 100ms timeout intent Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent d1c0007 commit 3b3d649

File tree

4 files changed

+146
-2
lines changed

4 files changed

+146
-2
lines changed

lib/management/authz.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,7 @@ describe('Management Authz', () => {
613613
const targets = { targets: ['u1'] };
614614
fetchMock.mockResolvedValue({
615615
ok: true,
616+
text: async () => JSON.stringify(targets),
616617
json: async () => targets,
617618
clone: () => ({ json: async () => targets }),
618619
status: 200,
@@ -640,6 +641,7 @@ describe('Management Authz', () => {
640641
const relationsResponse = { relations: [mockRelation] };
641642
fetchMock.mockResolvedValue({
642643
ok: true,
644+
text: async () => JSON.stringify(relationsResponse),
643645
json: async () => relationsResponse,
644646
clone: () => ({ json: async () => relationsResponse }),
645647
status: 200,

lib/management/authz.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ const WithAuthz = (httpClient: HttpClient, config?: FGAConfig) => {
3939
});
4040

4141
if (response.ok) {
42+
// Pre-consume body to avoid node-fetch clone issues
43+
// (same workaround as fetchWrapper in core-js-sdk)
44+
const respText = await response.text();
45+
(response as any).text = () => Promise.resolve(respText);
46+
(response as any).json = async () => JSON.parse(respText);
47+
(response as any).clone = () => response;
4248
return response;
4349
}
4450
} catch {

lib/management/fga.test.ts

Lines changed: 132 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ describe('Management FGA', () => {
198198
const schema = { dsl: 'model AuthZ 1.0' };
199199
fetchMock.mockResolvedValue({
200200
ok: true,
201+
text: async () => JSON.stringify({}),
201202
json: async () => ({}),
202203
clone: () => ({ json: async () => ({}) }),
203204
status: 200,
@@ -223,6 +224,7 @@ describe('Management FGA', () => {
223224
const relations = [relation1];
224225
fetchMock.mockResolvedValue({
225226
ok: true,
227+
text: async () => JSON.stringify({}),
226228
json: async () => ({}),
227229
clone: () => ({ json: async () => ({}) }),
228230
status: 200,
@@ -248,6 +250,7 @@ describe('Management FGA', () => {
248250
const relations = [relation1];
249251
fetchMock.mockResolvedValue({
250252
ok: true,
253+
text: async () => JSON.stringify({}),
251254
json: async () => ({}),
252255
clone: () => ({ json: async () => ({}) }),
253256
status: 200,
@@ -271,10 +274,12 @@ describe('Management FGA', () => {
271274

272275
it('should use cache URL for check when configured', async () => {
273276
const relations = [relation1, relation2];
277+
const checkBody = { tuples: mockCheckResponseRelations };
274278
fetchMock.mockResolvedValue({
275279
ok: true,
276-
json: async () => ({ tuples: mockCheckResponseRelations }),
277-
clone: () => ({ json: async () => ({ tuples: mockCheckResponseRelations }) }),
280+
text: async () => JSON.stringify(checkBody),
281+
json: async () => checkBody,
282+
clone: () => ({ json: async () => checkBody }),
278283
status: 200,
279284
headers: new Map(),
280285
});
@@ -318,6 +323,7 @@ describe('Management FGA', () => {
318323
const customConfig = { ...fgaConfig, fgaCacheTimeoutMs: 60000 };
319324
fetchMock.mockResolvedValue({
320325
ok: true,
326+
text: async () => JSON.stringify({}),
321327
json: async () => ({}),
322328
clone: () => ({ json: async () => ({}) }),
323329
status: 200,
@@ -335,6 +341,7 @@ describe('Management FGA', () => {
335341
async (invalidValue) => {
336342
fetchMock.mockResolvedValue({
337343
ok: true,
344+
text: async () => JSON.stringify({}),
338345
json: async () => ({}),
339346
clone: () => ({ json: async () => ({}) }),
340347
status: 200,
@@ -347,5 +354,128 @@ describe('Management FGA', () => {
347354
expect(fetchMock).toHaveBeenCalled();
348355
},
349356
);
357+
358+
it('should return correct data for a large check response (1206 tuples) via cache path', async () => {
359+
const largeTuples = Array.from({ length: 1206 }, (_, i) => ({
360+
resource: `u${i}`,
361+
resourceType: 'user',
362+
relation: 'member',
363+
target: `g${i}`,
364+
targetType: 'group',
365+
allowed: i % 2 === 0,
366+
}));
367+
const responseBody = { tuples: largeTuples };
368+
const bodyText = JSON.stringify(responseBody);
369+
fetchMock.mockResolvedValue({
370+
ok: true,
371+
text: async () => bodyText,
372+
json: async () => responseBody,
373+
clone: () => ({ json: async () => responseBody }),
374+
status: 200,
375+
});
376+
377+
const result = await WithFGA(mockHttpClient, fgaConfig).check(
378+
largeTuples.map(({ resource, resourceType, relation, target, targetType }) => ({
379+
resource,
380+
resourceType,
381+
relation,
382+
target,
383+
targetType,
384+
})),
385+
);
386+
387+
expect(fetchMock).toHaveBeenCalled();
388+
expect(mockHttpClient.post).not.toHaveBeenCalled();
389+
expect(result.data).toHaveLength(1206);
390+
expect(result.data![0]).toEqual(largeTuples[0]);
391+
expect(result.data![1205]).toEqual(largeTuples[1205]);
392+
});
393+
394+
it('should handle 1000+ concurrent check calls via cache path', async () => {
395+
const singleTuple = [relation1];
396+
const responseBody = { tuples: [{ ...relation1, allowed: true }] };
397+
const bodyText = JSON.stringify(responseBody);
398+
fetchMock.mockImplementation(() =>
399+
Promise.resolve({
400+
ok: true,
401+
text: async () => bodyText,
402+
json: async () => responseBody,
403+
clone: () => ({ json: async () => responseBody }),
404+
status: 200,
405+
}),
406+
);
407+
408+
const calls = Array.from({ length: 1000 }, () =>
409+
WithFGA(mockHttpClient, fgaConfig).check(singleTuple),
410+
);
411+
const results = await Promise.all(calls);
412+
413+
expect(fetchMock).toHaveBeenCalledTimes(1000);
414+
expect(mockHttpClient.post).not.toHaveBeenCalled();
415+
results.forEach((result) => {
416+
expect(result.ok).toBe(true);
417+
expect(result.data).toHaveLength(1);
418+
expect(result.data![0].allowed).toBe(true);
419+
});
420+
});
421+
422+
// Regression test for the node-fetch clone hang this fix addresses.
423+
// Without the body pre-consumption fix, transformResponse calls clone().json() on the
424+
// raw node-fetch Response. When the body stream is already consumed, node-fetch's
425+
// clone().json() returns a Promise that never resolves — causing check() to hang
426+
// indefinitely with no timeout protection (the AbortController is already cleared).
427+
// With the fix, .clone() is overridden in postWithOptionalCache before transformResponse
428+
// sees it, so it always resolves immediately. The 100ms timeout is intentionally tight:
429+
// with the fix the call completes in <5ms; without it, it hangs indefinitely.
430+
it('should not hang when cache response .clone().json() never resolves (simulates node-fetch hang on consumed stream)', async () => {
431+
const responseBody = { tuples: [{ ...relation1, allowed: true }] };
432+
const bodyText = JSON.stringify(responseBody);
433+
fetchMock.mockResolvedValue({
434+
ok: true,
435+
text: async () => bodyText,
436+
json: async () => responseBody,
437+
// Simulate node-fetch: clone().json() on a consumed body stream hangs forever
438+
clone: () => ({ json: () => new Promise(() => {}) }),
439+
status: 200,
440+
});
441+
442+
// Without fix: hangs indefinitely (clone().json() never settles)
443+
// With fix: .clone() is overridden → resolves immediately from memoized body
444+
const result = await WithFGA(mockHttpClient, fgaConfig).check([relation1]);
445+
expect(result.ok).toBe(true);
446+
expect(result.data).toHaveLength(1);
447+
}, 100);
448+
449+
it('should not crash when cache response .clone() would throw (body pre-consumed)', async () => {
450+
const responseBody = { tuples: [{ ...relation1, allowed: true }] };
451+
const bodyText = JSON.stringify(responseBody);
452+
// Simulate a raw node-fetch response where .clone() would throw
453+
fetchMock.mockResolvedValue({
454+
ok: true,
455+
text: async () => bodyText,
456+
json: async () => responseBody,
457+
clone: () => {
458+
throw new Error('clone failed — body already consumed');
459+
},
460+
status: 200,
461+
});
462+
463+
// After the fix, .clone() is overridden before transformResponse sees it
464+
const result = await WithFGA(mockHttpClient, fgaConfig).check([relation1]);
465+
466+
expect(fetchMock).toHaveBeenCalled();
467+
expect(result.ok).toBe(true);
468+
expect(result.data).toHaveLength(1);
469+
});
470+
471+
it('should fall back to httpClient when cache returns non-OK status', async () => {
472+
fetchMock.mockResolvedValue({ ok: false, status: 503 });
473+
474+
const schema = { dsl: 'test' };
475+
await WithFGA(mockHttpClient, fgaConfig).saveSchema(schema);
476+
477+
expect(fetchMock).toHaveBeenCalled();
478+
expect(mockHttpClient.post).toHaveBeenCalledWith(apiPaths.fga.schema, schema);
479+
});
350480
});
351481
});

lib/management/fga.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ const WithFGA = (httpClient: HttpClient, config?: FGAConfig) => {
3737
});
3838

3939
if (response.ok) {
40+
// Pre-consume body to avoid node-fetch clone issues
41+
// (same workaround as fetchWrapper in core-js-sdk)
42+
const respText = await response.text();
43+
(response as any).text = () => Promise.resolve(respText);
44+
(response as any).json = async () => JSON.parse(respText);
45+
(response as any).clone = () => response;
4046
return response;
4147
}
4248
} catch {

0 commit comments

Comments
 (0)