diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index ae41e755..9bb47dd1 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -26,190 +26,190 @@ jobs: SERVERLESS_INDEX_NAME=$(npx ts-node ./src/integration/setup.ts | grep "SERVERLESS_INDEX_NAME=" | cut -d'=' -f2) echo "SERVERLESS_INDEX_NAME=$SERVERLESS_INDEX_NAME" >> $GITHUB_OUTPUT - unit-tests: - needs: setup - name: Run unit tests - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Setup - uses: ./.github/actions/setup - - - name: Run tests - env: - CI: true - run: npm run test:unit -- --coverage - - integration-tests: - needs: setup - name: Run integration tests - runs-on: ubuntu-latest - outputs: - serverlessIndexName: ${{ steps.runTests1.outputs.SERVERLESS_INDEX_NAME }} - strategy: - fail-fast: false - max-parallel: 2 - matrix: - pinecone_env: - - prod - # - staging - node_version: [18.x, 20.x] - config: - [ - { runner: 'npm', jest_env: 'node' }, - { runner: 'npm', jest_env: 'edge' }, - { runner: 'bun', jest_env: 'node', bun_version: '1.0.0' }, - { runner: 'bun', jest_env: 'node', bun_version: '1.0.36' }, - { runner: 'bun', jest_env: 'node', bun_version: '1.1.11' }, - ] - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Setup - uses: ./.github/actions/setup - with: - node_version: ${{ matrix.node_version }} - - - name: Setup bun - if: matrix.config.bun_version - uses: oven-sh/setup-bun@v1 - with: - bun-version: ${{ matrix.config.bun_version }} - - - name: Run integration tests (Prod) - id: runTests1 - if: matrix.pinecone_env == 'prod' - env: - CI: true - PINECONE_API_KEY: ${{ secrets.PINECONE_API_KEY }} - SERVERLESS_INDEX_NAME: ${{ needs.setup.outputs.serverlessIndexName}} - run: | - ${{ matrix.config.runner }} run test:integration:${{ matrix.config.jest_env }} - echo "SERVERLESS_INDEX_NAME=${{ needs.setup.outputs.serverlessIndexName}}" >> $GITHUB_OUTPUT - - - name: Run integration tests (Staging) - if: matrix.pinecone_env == 'staging' - env: - CI: true - PINECONE_API_KEY: ${{ secrets.PINECONE_API_KEY }} - PINECONE_CONTROLLER_HOST: 'https://api-staging.pinecone.io' - run: ${{ matrix.config.runner }} run test:integration:${{ matrix.config.jest_env }} - - teardown: - name: Teardown - needs: integration-tests # Ensure teardown only runs after test jobs - runs-on: ubuntu-latest - if: always() # Run teardown even if the tests fail - steps: - - name: Checkout code - uses: actions/checkout@v4 - - - name: Install dependencies - run: npm ci - - - name: Run teardown script - env: - PINECONE_API_KEY: ${{ secrets.PINECONE_API_KEY }} - SERVERLESS_INDEX_NAME: ${{ needs.integration-tests.outputs.serverlessIndexName}} - run: | - npx ts-node ./src/integration/teardown.ts - - typescript-compilation-tests: - name: TS compile - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - tsVersion: - [ - '~4.1.0', - '~4.2.0', - '~4.3.0', - '~4.4.0', - '~4.5.0', - '~4.6.0', - '~4.7.0', - '~4.8.0', - '~4.9.0', - '~5.0.0', - '~5.1.0', - '~5.2.0', - 'latest', - ] - steps: - - name: Checkout - uses: actions/checkout@v4 - - name: Setup Node - uses: actions/setup-node@v4 - with: - node-version: 18.x - registry-url: 'https://registry.npmjs.org' - - name: Install npm packages - run: | - npm install --ignore-scripts - shell: bash - - name: Build TypeScript for Pinecone - run: npm run build - shell: bash - - name: install, compile, and run sample app - shell: bash - env: - PINECONE_API_KEY: ${{ secrets.PINECONE_API_KEY }} - run: | - set -x - cd .. - cp -r pinecone-ts-client/ts-compilation-test ts-compilation-test - cd ts-compilation-test - npm install typescript@${{ matrix.tsVersion }} --no-cache - npm install '../pinecone-ts-client' --no-cache - cat package.json - cat package-lock.json - npm run tsc-version - npm run build - npm run start - - example-app-semantic-search: - name: 'Example app: semantic search' - runs-on: ubuntu-latest - steps: - - name: Checkout pinecone-ts-client - uses: actions/checkout@v4 - with: - path: pinecone-ts-client - - name: Checkout semantic-search-example - uses: actions/checkout@v4 - with: - repository: pinecone-io/semantic-search-example - ref: spruce - path: semantic-search-example - - name: Install and build client - shell: bash - run: | - cd pinecone-ts-client - npm install --ignore-scripts - npm run build - - name: Install and build sample app - shell: bash - run: | - cd semantic-search-example - npm install --no-cache - npm install '../pinecone-ts-client' - cat package.json - - name: Run example tests with dev version of client - env: - CI: true - PINECONE_API_KEY: ${{ secrets.PINECONE_API_KEY }} - PINECONE_INDEX: 'semantic-search' - PINECONE_CLOUD: 'aws' - PINECONE_REGION: 'us-west-2' - shell: bash - run: | - cd semantic-search-example - npm run test + unit-tests: + needs: setup + name: Run unit tests + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup + uses: ./.github/actions/setup + + - name: Run tests + env: + CI: true + run: npm run test:unit -- --coverage + + integration-tests: + needs: setup + name: Run integration tests + runs-on: ubuntu-latest + outputs: + serverlessIndexName: ${{ steps.runTests1.outputs.SERVERLESS_INDEX_NAME }} + strategy: + fail-fast: false + max-parallel: 2 + matrix: + pinecone_env: + - prod + # - staging + node_version: [18.x, 20.x] + config: + [ + { runner: 'npm', jest_env: 'node' }, + { runner: 'npm', jest_env: 'edge' }, + { runner: 'bun', jest_env: 'node', bun_version: '1.0.0' }, + { runner: 'bun', jest_env: 'node', bun_version: '1.0.36' }, + { runner: 'bun', jest_env: 'node', bun_version: '1.1.11' }, + ] + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup + uses: ./.github/actions/setup + with: + node_version: ${{ matrix.node_version }} + + - name: Setup bun + if: matrix.config.bun_version + uses: oven-sh/setup-bun@v1 + with: + bun-version: ${{ matrix.config.bun_version }} + + - name: Run integration tests (Prod) + id: runTests1 + if: matrix.pinecone_env == 'prod' + env: + CI: true + PINECONE_API_KEY: ${{ secrets.PINECONE_API_KEY }} + SERVERLESS_INDEX_NAME: ${{ needs.setup.outputs.serverlessIndexName}} + run: | + ${{ matrix.config.runner }} run test:integration:${{ matrix.config.jest_env }} + echo "SERVERLESS_INDEX_NAME=${{ needs.setup.outputs.serverlessIndexName}}" >> $GITHUB_OUTPUT + + - name: Run integration tests (Staging) + if: matrix.pinecone_env == 'staging' + env: + CI: true + PINECONE_API_KEY: ${{ secrets.PINECONE_API_KEY }} + PINECONE_CONTROLLER_HOST: 'https://api-staging.pinecone.io' + run: ${{ matrix.config.runner }} run test:integration:${{ matrix.config.jest_env }} + + teardown: + name: Teardown + needs: integration-tests # Ensure teardown only runs after test jobs + runs-on: ubuntu-latest + if: always() # Run teardown even if the tests fail + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Install dependencies + run: npm ci + + - name: Run teardown script + env: + PINECONE_API_KEY: ${{ secrets.PINECONE_API_KEY }} + SERVERLESS_INDEX_NAME: ${{ needs.integration-tests.outputs.serverlessIndexName}} + run: | + npx ts-node ./src/integration/teardown.ts + + typescript-compilation-tests: + name: TS compile + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + tsVersion: + [ + '~4.1.0', + '~4.2.0', + '~4.3.0', + '~4.4.0', + '~4.5.0', + '~4.6.0', + '~4.7.0', + '~4.8.0', + '~4.9.0', + '~5.0.0', + '~5.1.0', + '~5.2.0', + 'latest', + ] + steps: + - name: Checkout + uses: actions/checkout@v4 + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version: 18.x + registry-url: 'https://registry.npmjs.org' + - name: Install npm packages + run: | + npm install --ignore-scripts + shell: bash + - name: Build TypeScript for Pinecone + run: npm run build + shell: bash + - name: install, compile, and run sample app + shell: bash + env: + PINECONE_API_KEY: ${{ secrets.PINECONE_API_KEY }} + run: | + set -x + cd .. + cp -r pinecone-ts-client/ts-compilation-test ts-compilation-test + cd ts-compilation-test + npm install typescript@${{ matrix.tsVersion }} --no-cache + npm install '../pinecone-ts-client' --no-cache + cat package.json + cat package-lock.json + npm run tsc-version + npm run build + npm run start + + example-app-semantic-search: + name: 'Example app: semantic search' + runs-on: ubuntu-latest + steps: + - name: Checkout pinecone-ts-client + uses: actions/checkout@v4 + with: + path: pinecone-ts-client + - name: Checkout semantic-search-example + uses: actions/checkout@v4 + with: + repository: pinecone-io/semantic-search-example + ref: spruce + path: semantic-search-example + - name: Install and build client + shell: bash + run: | + cd pinecone-ts-client + npm install --ignore-scripts + npm run build + - name: Install and build sample app + shell: bash + run: | + cd semantic-search-example + npm install --no-cache + npm install '../pinecone-ts-client' + cat package.json + - name: Run example tests with dev version of client + env: + CI: true + PINECONE_API_KEY: ${{ secrets.PINECONE_API_KEY }} + PINECONE_INDEX: 'semantic-search' + PINECONE_CLOUD: 'aws' + PINECONE_REGION: 'us-west-2' + shell: bash + run: | + cd semantic-search-example + npm run test external-app: name: external-app diff --git a/README.md b/README.md index d5a30704..5d516c18 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ const pc = new Pinecone(); If you prefer to pass configuration in code, the constructor accepts a config object containing the `apiKey` value. This is the object in which you would pass properties like `maxRetries` (defaults to `3`) for retryable operations -(e.g. `upsert`). +(`upsert`, `update`, and `configureIndex`). ```typescript import { Pinecone } from '@pinecone-database/pinecone'; diff --git a/src/control/configureIndex.ts b/src/control/configureIndex.ts index eaf94279..4aa64719 100644 --- a/src/control/configureIndex.ts +++ b/src/control/configureIndex.ts @@ -6,6 +6,7 @@ import { import { PineconeArgumentError } from '../errors'; import type { IndexName } from './types'; import { ValidateProperties } from '../utils/validateProperties'; +import { RetryOnServerFailure } from '../utils'; // Properties for validation to ensure no unknown/invalid properties are passed, no req'd properties are missing type ConfigureIndexRequestType = keyof ConfigureIndexRequest; @@ -49,11 +50,17 @@ export const configureIndex = (api: ManageIndexesApi) => { return async ( indexName: IndexName, - options: ConfigureIndexRequest + options: ConfigureIndexRequest, + maxRetries?: number ): Promise => { validator(indexName, options); - return await api.configureIndex({ + const retryWrapper = new RetryOnServerFailure( + api.configureIndex.bind(api), + maxRetries + ); + + return await retryWrapper.execute({ indexName, configureIndexRequest: options, }); diff --git a/src/data/index.ts b/src/data/index.ts index 7de8dce7..f21e5f87 100644 --- a/src/data/index.ts +++ b/src/data/index.ts @@ -536,7 +536,7 @@ export class Index { * @returns A promise that resolves when the update is completed. */ async update(options: UpdateOptions) { - return await this._updateCommand.run(options); + return await this._updateCommand.run(options, this.config.maxRetries); } /** diff --git a/src/data/vectors/update.ts b/src/data/vectors/update.ts index 7a586c68..aa9f1186 100644 --- a/src/data/vectors/update.ts +++ b/src/data/vectors/update.ts @@ -7,6 +7,7 @@ import type { } from './types'; import { PineconeArgumentError } from '../../errors'; import { ValidateProperties } from '../../utils/validateProperties'; +import { RetryOnServerFailure } from '../../utils'; /** * This type is very similar to { @link PineconeRecord }, but differs because the @@ -62,7 +63,7 @@ export class UpdateCommand { } }; - async run(options: UpdateOptions): Promise { + async run(options: UpdateOptions, maxRetries?: number): Promise { this.validator(options); const requestOptions = { @@ -73,7 +74,12 @@ export class UpdateCommand { }; const api = await this.apiProvider.provide(); - await api.updateVector({ + const retryWrapper = new RetryOnServerFailure( + api.updateVector.bind(api), + maxRetries + ); + + await retryWrapper.execute({ updateRequest: { ...requestOptions, namespace: this.namespace }, }); return; diff --git a/src/errors/http.ts b/src/errors/http.ts index f229b3c0..0573b758 100644 --- a/src/errors/http.ts +++ b/src/errors/http.ts @@ -198,6 +198,8 @@ export const mapHttpStatusError = (failedRequestInfo: FailedRequestInfo) => { return new PineconeInternalServerError(failedRequestInfo); case 501: return new PineconeNotImplementedError(failedRequestInfo); + case 503: + return new PineconeUnavailableError(failedRequestInfo); default: throw new PineconeUnmappedHttpError(failedRequestInfo); } diff --git a/src/integration/control/configureIndex.test.ts b/src/integration/control/configureIndex.test.ts index 1f203b9f..846acb9d 100644 --- a/src/integration/control/configureIndex.test.ts +++ b/src/integration/control/configureIndex.test.ts @@ -1,15 +1,6 @@ -import { - BasePineconeError, - PineconeBadRequestError, - PineconeInternalServerError, -} from '../../errors'; +import { BasePineconeError, PineconeBadRequestError } from '../../errors'; import { Pinecone } from '../../index'; -import { - randomIndexName, - retryDeletes, - sleep, - waitUntilReady, -} from '../test-helpers'; +import { randomIndexName, retryDeletes, waitUntilReady } from '../test-helpers'; let podIndexName: string, serverlessIndexName: string, pinecone: Pinecone; @@ -75,26 +66,10 @@ describe('configure index', () => { const description = await pinecone.describeIndex(podIndexName); expect(description.spec.pod?.podType).toEqual('p1.x1'); - // Scale up podType to x2 - let state = true; - let retryCount = 0; - const maxRetries = 10; - while (state && retryCount < maxRetries) { - try { - await pinecone.configureIndex(podIndexName, { - spec: { pod: { podType: 'p1.x2' } }, - }); - state = false; - } catch (e) { - if (e instanceof PineconeInternalServerError) { - retryCount++; - await sleep(2000); - } else { - console.log('Unexpected error:', e); - throw e; - } - } - } + await pinecone.configureIndex(podIndexName, { + spec: { pod: { podType: 'p1.x2' } }, + }); + await waitUntilReady(podIndexName); const description2 = await pinecone.describeIndex(podIndexName); expect(description2.spec.pod?.podType).toEqual('p1.x2'); diff --git a/src/integration/data/vectors/upsertAndUpdate.test.ts b/src/integration/data/vectors/upsertAndUpdate.test.ts index 7ed52683..03deb1fa 100644 --- a/src/integration/data/vectors/upsertAndUpdate.test.ts +++ b/src/integration/data/vectors/upsertAndUpdate.test.ts @@ -6,6 +6,7 @@ import { generateRecords, globalNamespaceOne, randomIndexName, + sleep, waitUntilReady, } from '../../test-helpers'; import { RetryOnServerFailure } from '../../../utils'; @@ -85,7 +86,7 @@ describe('upsert and update to serverless index', () => { }); // Retry logic tests -describe('Testing retry logic on Upsert operation, as run on a mock, in-memory http server', () => { +describe('Testing retry logic via a mock, in-memory http server', () => { const recordsToUpsert = generateRecords({ dimension: 2, quantity: 1, @@ -96,13 +97,14 @@ describe('Testing retry logic on Upsert operation, as run on a mock, in-memory h let server: http.Server; // Note: server cannot be something like an express server due to conflicts w/edge runtime let mockServerlessIndex: Index; let callCount: number; + let op: string; // Helper function to start the server with a specific response pattern const startMockServer = (shouldSucceedOnSecondCall: boolean) => { // Create http server server = http.createServer((req, res) => { const { pathname } = parse(req.url || '', true); - if (req.method === 'POST' && pathname === '/vectors/upsert') { + if (req.method === 'POST' && pathname === `/vectors/${op}`) { callCount++; if (shouldSucceedOnSecondCall && callCount === 1) { res.writeHead(503, { 'Content-Type': 'application/json' }); @@ -137,6 +139,7 @@ describe('Testing retry logic on Upsert operation, as run on a mock, in-memory h }); test('Upsert operation should retry 1x if server responds 1x with error and 1x with success', async () => { + op = 'upsert'; pinecone = new Pinecone({ apiKey: process.env['PINECONE_API_KEY'] || '', maxRetries: 2, @@ -155,6 +158,39 @@ describe('Testing retry logic on Upsert operation, as run on a mock, in-memory h // Call Upsert operation await mockServerlessIndex.upsert(recordsToUpsert); + // 2 total tries: 1 initial call, 1 retry + expect(retrySpy).toHaveBeenCalledTimes(1); // passes + expect(delaySpy).toHaveBeenCalledTimes(1); // fails + expect(callCount).toBe(2); + }); + + test('Update operation should retry 1x if server responds 1x with error and 1x with success', async () => { + op = 'update'; + + pinecone = new Pinecone({ + apiKey: process.env['PINECONE_API_KEY'] || '', + maxRetries: 2, + }); + + mockServerlessIndex = pinecone + .Index(serverlessIndexName, 'http://localhost:4000') + .namespace(globalNamespaceOne); + + const retrySpy = jest.spyOn(RetryOnServerFailure.prototype, 'execute'); + const delaySpy = jest.spyOn(RetryOnServerFailure.prototype, 'delay'); + + // Start server with a successful response on the second call + startMockServer(true); + + const recordIdToUpdate = recordsToUpsert[0].id; + const newMetadata = { flavor: 'chocolate' }; + + // Call Update operation + await mockServerlessIndex.update({ + id: recordIdToUpdate, + metadata: newMetadata, + }); + // 2 total tries: 1 initial call, 1 retry expect(retrySpy).toHaveBeenCalledTimes(1); expect(delaySpy).toHaveBeenCalledTimes(1); @@ -162,6 +198,10 @@ describe('Testing retry logic on Upsert operation, as run on a mock, in-memory h }); test('Max retries exceeded w/o resolve', async () => { + op = 'upsert'; + + await sleep(500); // In Node20+, tcp connections changed: https://github.com/pinecone-io/pinecone-ts-client/pull/318#issuecomment-2560180936 + pinecone = new Pinecone({ apiKey: process.env['PINECONE_API_KEY'] || '', maxRetries: 3, diff --git a/src/pinecone.ts b/src/pinecone.ts index ee270e63..5fde1965 100644 --- a/src/pinecone.ts +++ b/src/pinecone.ts @@ -472,7 +472,7 @@ export class Pinecone { * @returns A promise that resolves to {@link IndexModel} when the request to configure the index is completed. */ configureIndex(indexName: IndexName, options: ConfigureIndexRequest) { - return this._configureIndex(indexName, options); + return this._configureIndex(indexName, options, this.config.maxRetries); } /** diff --git a/src/utils/__tests__/retries.test.ts b/src/utils/__tests__/retries.test.ts index cd0f1d21..e34f176f 100644 --- a/src/utils/__tests__/retries.test.ts +++ b/src/utils/__tests__/retries.test.ts @@ -112,3 +112,41 @@ describe('isRetryError', () => { expect(result).toBe(true); }); }); + +describe('shouldStopRetrying', () => { + test('Should return true for non-retryable status code', () => { + const retryWrapper = new RetryOnServerFailure(() => Promise.resolve({})); + const result = retryWrapper['shouldStopRetrying']({ status: 400 }); + expect(result).toBe(true); + }); + + test('Should return true for non-retryable error name', () => { + const retryWrapper = new RetryOnServerFailure(() => Promise.resolve({})); + const result = retryWrapper['shouldStopRetrying']({ + name: 'SomeOtherError', + }); + expect(result).toBe(true); + }); + + test('Should return false for retryable error name PineconeUnavailableError', () => { + const retryWrapper = new RetryOnServerFailure(() => Promise.resolve({})); + const result = retryWrapper['shouldStopRetrying']({ + name: 'PineconeUnavailableError', + }); + expect(result).toBe(false); + }); + + test('Should return false for retryable error name PineconeInternalServerError', () => { + const retryWrapper = new RetryOnServerFailure(() => Promise.resolve({})); + const result = retryWrapper['shouldStopRetrying']({ + name: 'PineconeInternalServerError', + }); + expect(result).toBe(false); + }); + + test('Should return false for retryable status code', () => { + const retryWrapper = new RetryOnServerFailure(() => Promise.resolve({})); + const result = retryWrapper['shouldStopRetrying']({ status: 500 }); + expect(result).toBe(false); + }); +}); diff --git a/src/utils/retries.ts b/src/utils/retries.ts index 1a4d3b4b..34b7d9fa 100644 --- a/src/utils/retries.ts +++ b/src/utils/retries.ts @@ -1,4 +1,4 @@ -import { PineconeMaxRetriesExceededError } from '../errors'; +import { mapHttpStatusError, PineconeMaxRetriesExceededError } from '../errors'; /* Retry asynchronous operations. * @@ -24,21 +24,39 @@ export class RetryOnServerFailure { } async execute(...args: A): Promise { + if (this.maxRetries < 1) { + return this.asyncFn(...args); + } + for (let attempt = 0; attempt < this.maxRetries; attempt++) { try { const response = await this.asyncFn(...args); + + // Return immediately if the response is not a retryable error if (!this.isRetryError(response)) { - return response; // Return if there's no retryable error + return response; } - throw response; // Throw if response is retryable, and catch below + + throw response; // Will catch this in next line } catch (error) { + const mappedError = this.mapErrorIfNeeded(error); + + // If the error is not retryable, throw it immediately + if (this.shouldStopRetrying(mappedError)) { + throw mappedError; + } + + // On the last retry, throw a MaxRetriesExceededError if (attempt === this.maxRetries - 1) { throw new PineconeMaxRetriesExceededError(this.maxRetries); } - await this.delay(attempt + 1); // Increment before passing to `delay` + + // Wait before retrying + await this.delay(attempt + 1); } } - // Fallback throw in case no value is successfully returned in order to comply w/return type Promise + + // This fallback is unnecessary, but included for type safety throw new PineconeMaxRetriesExceededError(this.maxRetries); } @@ -88,4 +106,24 @@ export class RetryOnServerFailure { // Ensure delay is not negative or greater than maxDelay return Math.min(maxDelay, Math.max(0, delay)); }; + + private mapErrorIfNeeded(error: any): any { + if (error?.status) { + return mapHttpStatusError(error); + } + return error; // Return original error if no mapping is needed + } + + private shouldStopRetrying(error: any): boolean { + if (error.status) { + return error.status < 500; + } + if (error.name) { + return ( + error.name !== 'PineconeUnavailableError' && + error.name !== 'PineconeInternalServerError' + ); + } + return true; + } }