Skip to content

Commit 8bbcfe2

Browse files
jonathanMLDevzho
andauthored
Wire request timeout and 429/5xx retry onto Pinecone I/O hot path (#225)
* Wire request timeout and 429/5xx retry onto Pinecone I/O hot path * Propagated app timeout errors from hybrid search. * fixed some nit findings * fixed format error * add mapPc function * - keep the SDK methods inside the guard - added some tests --------- Co-authored-by: zho <jornathanm910923@gmail.com>
1 parent 98d0164 commit 8bbcfe2

12 files changed

Lines changed: 602 additions & 108 deletions

src/core/pinecone-client.test.ts

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, it, expect, beforeEach, vi } from 'vitest';
1+
import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest';
22
import { PineconeClient } from './pinecone-client.js';
33
import { resolveConfig } from './config.js';
44
import type { SearchableIndex, PineconeHit } from '../types.js';
@@ -21,6 +21,16 @@ function stubPineconeClient(client: PineconeClient): PineconeClientMethodStubs {
2121
return client as unknown as PineconeClientMethodStubs;
2222
}
2323

24+
function stubDualLegSearchFailure(testClient: PineconeClientMethodStubs, searchError: Error): void {
25+
testClient.ensureIndexes = async () => ({
26+
denseIndex: {} as SearchableIndex,
27+
sparseIndex: {} as SearchableIndex,
28+
});
29+
testClient.searchIndex = async () => {
30+
throw searchError;
31+
};
32+
}
33+
2434
describe('PineconeClient', () => {
2535
let client: PineconeClient;
2636

@@ -32,6 +42,10 @@ describe('PineconeClient', () => {
3242
});
3343
});
3444

45+
afterEach(() => {
46+
vi.useRealTimers();
47+
});
48+
3549
describe('constructor', () => {
3650
it('should initialize with provided config', () => {
3751
expect(client).toBeDefined();
@@ -143,14 +157,7 @@ describe('PineconeClient', () => {
143157

144158
it('should throw when both dense and sparse searches fail', async () => {
145159
const testClient = stubPineconeClient(client);
146-
147-
testClient.ensureIndexes = async () => ({
148-
denseIndex: {} as SearchableIndex,
149-
sparseIndex: {} as SearchableIndex,
150-
});
151-
testClient.searchIndex = async () => {
152-
throw new Error('index failure');
153-
};
160+
stubDualLegSearchFailure(testClient, new Error('index failure'));
154161

155162
await expect(
156163
client.query({
@@ -161,6 +168,23 @@ describe('PineconeClient', () => {
161168
})
162169
).rejects.toThrow('Hybrid search failed: both dense and sparse index searches failed.');
163170
});
171+
172+
it('propagates app timeout when both dense and sparse searches fail', async () => {
173+
const testClient = stubPineconeClient(client);
174+
stubDualLegSearchFailure(
175+
testClient,
176+
new Error('Timeout after 50ms while waiting for search')
177+
);
178+
179+
await expect(
180+
client.query({
181+
query: 'hybrid search',
182+
namespace: 'test',
183+
topK: 5,
184+
useReranking: false,
185+
})
186+
).rejects.toThrow('Timeout after 50ms while waiting for search');
187+
});
164188
});
165189

166190
describe('count', () => {
@@ -443,6 +467,27 @@ describe('PineconeClient', () => {
443467
);
444468
});
445469

470+
it('passes configured requestTimeoutMs to search call sites', async () => {
471+
vi.useFakeTimers();
472+
const timeoutClient = new PineconeClient({
473+
apiKey: 'test-api-key',
474+
indexName: 'test-index',
475+
requestTimeoutMs: 50,
476+
});
477+
const testClient = stubPineconeClient(timeoutClient);
478+
const search = vi.fn(() => new Promise(() => {}));
479+
const sparseRef = { search } as SearchableIndex;
480+
testClient.ensureIndexes = async () => ({
481+
denseIndex: {} as SearchableIndex,
482+
sparseIndex: sparseRef,
483+
});
484+
485+
const p = timeoutClient.keywordSearch({ query: 'q', namespace: 'n' });
486+
const assertion = expect(p).rejects.toThrow(/Timeout after 50ms while waiting for search/);
487+
await vi.advanceTimersByTimeAsync(50);
488+
await assertion;
489+
});
490+
446491
it('searches sparse index only and maps hits', async () => {
447492
const testClient = stubPineconeClient(client);
448493
const denseRef = {} as SearchableIndex;

src/core/pinecone-client.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
sliceMergedHitsToSearchResults,
2626
} from './pinecone/search.js';
2727
import { rerankResults as rerankResultsImpl } from './pinecone/rerank.js';
28+
import { isAppTimeoutError } from './server/retry.js';
2829

2930
export class PineconeClient {
3031
private readonly rerankModel: string | undefined;
@@ -105,7 +106,15 @@ export class PineconeClient {
105106
metadataFilter?: Record<string, unknown>,
106107
options?: { fields?: string[] }
107108
): Promise<PineconeHit[]> {
108-
return searchIndexImpl(index, query, topK, namespace, metadataFilter, options);
109+
return searchIndexImpl(
110+
index,
111+
query,
112+
topK,
113+
namespace,
114+
metadataFilter,
115+
options,
116+
this.indexSession.getRequestTimeoutMs()
117+
);
109118
}
110119

111120
async query(params: QueryParams): Promise<HybridQueryResult> {
@@ -149,6 +158,8 @@ export class PineconeClient {
149158
logError('Sparse index search failed', sparseResult.reason);
150159
}
151160
if (denseResult.status === 'rejected' && sparseResult.status === 'rejected') {
161+
if (isAppTimeoutError(denseResult.reason)) throw denseResult.reason;
162+
if (isAppTimeoutError(sparseResult.reason)) throw sparseResult.reason;
152163
throw new Error('Hybrid search failed: both dense and sparse index searches failed.');
153164
}
154165

@@ -179,7 +190,8 @@ export class PineconeClient {
179190
this.rerankModel,
180191
query,
181192
mergedResults,
182-
topK
193+
topK,
194+
this.indexSession.getRequestTimeoutMs()
183195
);
184196
documents = rerankOut.results;
185197
degraded = rerankOut.degraded;

src/core/pinecone/indexes.test.ts

Lines changed: 197 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, it, expect, vi } from 'vitest';
1+
import { describe, it, expect, vi, afterEach } from 'vitest';
22
import { PineconeIndexSession } from './indexes.js';
33
import type { SearchableIndex } from '../../types.js';
44

@@ -29,6 +29,60 @@ class ThrowingEnsureSession extends PineconeIndexSession {
2929
}
3030
}
3131

32+
/** Session with a short timeout for fake-timer I/O deadline tests. */
33+
class TimeoutIndexSession extends PineconeIndexSession {
34+
constructor(
35+
private readonly pair: { dense: SearchableIndex; sparse: SearchableIndex },
36+
requestTimeoutMs = 50
37+
) {
38+
super('test-api-key', 'test-index', undefined, requestTimeoutMs);
39+
}
40+
41+
override async ensureIndexes(): Promise<{
42+
denseIndex: SearchableIndex;
43+
sparseIndex: SearchableIndex;
44+
}> {
45+
return { denseIndex: this.pair.dense, sparseIndex: this.pair.sparse };
46+
}
47+
}
48+
49+
afterEach(() => {
50+
vi.useRealTimers();
51+
});
52+
53+
/** Mock describeIndexStats that fails when the SDK method is invoked detached. */
54+
function makeThisSensitiveIndex(
55+
result: {
56+
namespaces?: Record<string, { recordCount?: number }>;
57+
dimension?: number;
58+
} = {}
59+
): SearchableIndex {
60+
const index = {
61+
describeIndexStats() {
62+
if (this !== index) {
63+
throw new TypeError('detached SDK method: wrong this binding');
64+
}
65+
return Promise.resolve(result);
66+
},
67+
};
68+
return index as unknown as SearchableIndex;
69+
}
70+
71+
/** Mock namespace().query that fails when the SDK method is invoked detached. */
72+
function makeThisSensitiveNamespaceHandle(
73+
matches: Array<{ metadata?: Record<string, unknown> }> = []
74+
) {
75+
const handle = {
76+
query() {
77+
if (this !== handle) {
78+
throw new TypeError('detached SDK method: wrong this binding');
79+
}
80+
return Promise.resolve({ matches });
81+
},
82+
};
83+
return handle;
84+
}
85+
3286
describe('PineconeIndexSession', () => {
3387
describe('listNamespacesFromKeywordIndex', () => {
3488
it('returns namespace rows when describeIndexStats succeeds', async () => {
@@ -66,6 +120,63 @@ describe('PineconeIndexSession', () => {
66120
expect(result.error).toContain('stats unavailable');
67121
}
68122
});
123+
124+
it('retries describeIndexStats on 503 then succeeds', async () => {
125+
let n = 0;
126+
const sparse = {
127+
describeIndexStats: vi.fn().mockImplementation(async () => {
128+
n++;
129+
if (n < 2) throw new Error('HTTP 503');
130+
return { namespaces: { papers: { recordCount: 7 } } };
131+
}),
132+
} as unknown as SearchableIndex;
133+
const session = new PineconeIndexSessionTestDouble({
134+
dense: {} as SearchableIndex,
135+
sparse,
136+
});
137+
138+
const result = await session.listNamespacesFromKeywordIndex();
139+
140+
expect(result.ok).toBe(true);
141+
if (result.ok) {
142+
expect(result.namespaces).toEqual([{ namespace: 'papers', recordCount: 7 }]);
143+
}
144+
expect(sparse.describeIndexStats).toHaveBeenCalledTimes(2);
145+
});
146+
147+
it('invokes describeIndexStats on the sparse index receiver through runIo', async () => {
148+
const sparse = makeThisSensitiveIndex({
149+
namespaces: { papers: { recordCount: 42 } },
150+
});
151+
const session = new PineconeIndexSessionTestDouble({
152+
dense: {} as SearchableIndex,
153+
sparse,
154+
});
155+
156+
const result = await session.listNamespacesFromKeywordIndex();
157+
158+
expect(result.ok).toBe(true);
159+
if (result.ok) {
160+
expect(result.namespaces).toEqual([{ namespace: 'papers', recordCount: 42 }]);
161+
}
162+
});
163+
164+
it('times out describeIndexStats at requestTimeoutMs', async () => {
165+
vi.useFakeTimers();
166+
const sparse = {
167+
describeIndexStats: vi.fn(() => new Promise(() => {})),
168+
} as unknown as SearchableIndex;
169+
const session = new TimeoutIndexSession({ dense: {} as SearchableIndex, sparse });
170+
const p = session.listNamespacesFromKeywordIndex();
171+
const assertion = expect(p).resolves.toMatchObject({
172+
ok: false,
173+
error: expect.stringMatching(
174+
/Timeout after 50ms while waiting for describeIndexStats-sparse/
175+
),
176+
});
177+
await vi.advanceTimersByTimeAsync(50);
178+
await assertion;
179+
});
69180
});
70181

71182
describe('listNamespacesWithMetadata', () => {
@@ -105,6 +216,28 @@ describe('PineconeIndexSession', () => {
105216
});
106217
});
107218

219+
it('invokes namespace().query on the namespace receiver through runIo when sampling', async () => {
220+
const nsHandle = makeThisSensitiveNamespaceHandle([
221+
{ metadata: { title: 'T', tags: ['a'] } },
222+
]);
223+
const dense = makeThisSensitiveIndex({
224+
namespaces: { ns1: { recordCount: 2 } },
225+
dimension: 4,
226+
}) as SearchableIndex & { namespace: (name: string) => typeof nsHandle };
227+
dense.namespace = () => nsHandle;
228+
const session = new PineconeIndexSessionTestDouble({
229+
dense,
230+
sparse: {} as SearchableIndex,
231+
});
232+
233+
const rows = await session.listNamespacesWithMetadata();
234+
expect(rows.namespaces).toHaveLength(1);
235+
expect(rows.namespaces[0]?.metadata).toMatchObject({
236+
title: 'string',
237+
tags: 'string[]',
238+
});
239+
});
240+
108241
it('samples metadata when records exist and namespace.query returns matches', async () => {
109242
const dense = {
110243
describeIndexStats: vi.fn().mockResolvedValue({
@@ -141,6 +274,33 @@ describe('PineconeIndexSession', () => {
141274
expect(rows.namespaces[0]?.schema_source).toBe('sampled');
142275
});
143276

277+
it('times out metadata sampling at requestTimeoutMs', async () => {
278+
vi.useFakeTimers();
279+
const dense = {
280+
describeIndexStats: vi.fn().mockResolvedValue({
281+
namespaces: { ns1: { recordCount: 2 } },
282+
dimension: 4,
283+
}),
284+
namespace: vi.fn().mockReturnValue({
285+
query: vi.fn(() => new Promise(() => {})),
286+
}),
287+
} as unknown as SearchableIndex;
288+
const session = new TimeoutIndexSession({ dense, sparse: {} as SearchableIndex });
289+
const p = session.listNamespacesWithMetadata();
290+
const assertion = expect(p).resolves.toMatchObject({
291+
namespaces: [
292+
{
293+
namespace: 'ns1',
294+
recordCount: 2,
295+
metadata: {},
296+
schema_source: 'sampled',
297+
},
298+
],
299+
});
300+
await vi.advanceTimersByTimeAsync(50);
301+
await assertion;
302+
});
303+
144304
it('uses declared schema and skips sampling when declaredSchemas provides one', async () => {
145305
const query = vi.fn();
146306
const dense = {
@@ -235,6 +395,16 @@ describe('PineconeIndexSession', () => {
235395
});
236396

237397
describe('checkIndexes', () => {
398+
it('invokes describeIndexStats on dense and sparse receivers through runIo', async () => {
399+
const dense = makeThisSensitiveIndex();
400+
const sparse = makeThisSensitiveIndex();
401+
const session = new PineconeIndexSessionTestDouble({ dense, sparse });
402+
403+
const result = await session.checkIndexes();
404+
expect(result.ok).toBe(true);
405+
expect(result.errors).toHaveLength(0);
406+
});
407+
238408
it('returns ok when describeIndexStats succeeds for dense and sparse', async () => {
239409
const dense = {
240410
describeIndexStats: vi.fn().mockResolvedValue({}),
@@ -348,5 +518,31 @@ describe('PineconeIndexSession', () => {
348518
/Timeout after 50ms while waiting for fetchRecordFields/
349519
);
350520
});
521+
522+
it('retries fetch on 503 then succeeds', async () => {
523+
let n = 0;
524+
class RetryFetchSession extends PineconeIndexSession {
525+
override ensureClient() {
526+
return {
527+
index: () => ({
528+
fetch: vi.fn().mockImplementation(async () => {
529+
n++;
530+
if (n < 2) throw new Error('HTTP 503');
531+
return {
532+
records: {
533+
schema_manifest: { metadata: { chunk_text: '{"ok":true}' } },
534+
},
535+
};
536+
}),
537+
}),
538+
} as never;
539+
}
540+
}
541+
542+
const session = new RetryFetchSession();
543+
const fields = await session.fetchRecordFields('_mcp_config', 'schema_manifest');
544+
expect(fields?.chunk_text).toBe('{"ok":true}');
545+
expect(n).toBe(2);
546+
});
351547
});
352548
});

0 commit comments

Comments
 (0)