Skip to content
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

Overhaul error handling to prevent unrelated text() error from masking underlying exceptions #135

Merged
merged 8 commits into from
Oct 9, 2023
Merged

Overhaul error handling to prevent unrelated text() error from masking underlying exceptions #135

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
808366c
Overhaul error handling
jhamon Oct 6, 2023
16ff144
For now, run only one integration test build at a time
jhamon Oct 6, 2023
0a1d8bf
Loosen up error messages to account for platform-specific differences
jhamon Oct 6, 2023
79643ab
Remove assertion about index state
jhamon Oct 7, 2023
8d1c351
Adjust configureIndex tests
jhamon Oct 7, 2023
05bce0f
Adjust describeIndex test
jhamon Oct 9, 2023
ff60217
Review feedback
jhamon Oct 9, 2023
37aabb0
Merge branch 'main' into jhamon/error-handling
jhamon Oct 9, 2023
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
1 change: 1 addition & 0 deletions .github/workflows/pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ jobs:
name: Run integration tests
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
jest_env: ['node', 'edge']
steps:
Expand Down
1 change: 1 addition & 0 deletions jest.config.integration-node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const config = require('./jest.config');

module.exports = {
...config,
reporters: [['github-actions', { silent: false }], 'default'],
setupFilesAfterEnv: ['./utils/globalIntegrationTestSetup.ts'],
testPathIgnorePatterns: [],
testEnvironment: 'node',
Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
"format": "prettier --write .",
"lint": "eslint src/ --ext .ts",
"repl": "npm run build && node utils/replInit.ts",
"test:integration:node": "jest src/integration/ -c jest.config.integration-node.js",
"test:integration:edge": "jest src/integration/ -c jest.config.integration-edge.js",
"test:integration:node": "TEST_ENV=node jest src/integration/ -c jest.config.integration-node.js --runInBand --bail",
"test:integration:edge": "TEST_ENV=edge jest src/integration/ -c jest.config.integration-edge.js --runInBand --bail",
"test:unit": "jest src/"
},
"engines": {
Expand Down
93 changes: 0 additions & 93 deletions src/control/__tests__/configureIndex.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import { configureIndex } from '../configureIndex';
import {
PineconeBadRequestError,
PineconeInternalServerError,
PineconeNotFoundError,
} from '../../errors';
import { IndexOperationsApi } from '../../pinecone-generated-ts-fetch';
import type { ConfigureIndexRequest } from '../../pinecone-generated-ts-fetch';

Expand All @@ -13,9 +8,6 @@ describe('configureIndex', () => {
jest.fn();
const IOA = { configureIndex: fakeConfigure } as IndexOperationsApi;

jest.mock('../../pinecone-generated-ts-fetch', () => ({
IndexOperationsApi: IOA,
}));
const returned = await configureIndex(IOA)('index-name', { replicas: 10 });

expect(returned).toBe(void 0);
Expand All @@ -24,89 +16,4 @@ describe('configureIndex', () => {
patchRequest: { replicas: 10 },
});
});

describe('http error mapping', () => {
test('when 500 occurs', async () => {
const fakeConfigure: (req: ConfigureIndexRequest) => Promise<string> =
jest.fn().mockImplementation(() =>
Promise.reject({
response: {
status: 500,
text: () => 'backend error message',
},
})
);
const IOA = {
configureIndex: fakeConfigure,
} as IndexOperationsApi;

jest.mock('../../pinecone-generated-ts-fetch', () => ({
IndexOperationsApi: IOA,
}));

const toThrow = async () => {
await configureIndex(IOA)('index-name', { replicas: 10 });
};

await expect(toThrow).rejects.toThrow(PineconeInternalServerError);
});

test('when 400 occurs, displays server message', async () => {
const fakeConfigure: (req: ConfigureIndexRequest) => Promise<string> =
jest.fn().mockImplementation(() =>
Promise.reject({
response: {
status: 400,
text: () => 'backend error message',
},
})
);
const IOA = {
configureIndex: fakeConfigure,
} as IndexOperationsApi;

jest.mock('../../pinecone-generated-ts-fetch', () => ({
IndexOperationsApi: IOA,
}));

const toThrow = async () => {
await configureIndex(IOA)('index-name', { replicas: 10 });
};

await expect(toThrow).rejects.toThrow(PineconeBadRequestError);
await expect(toThrow).rejects.toThrow('backend error message');
});

test('when 404 occurs, show available indexes', async () => {
const fakeConfigure: (req: ConfigureIndexRequest) => Promise<string> =
jest.fn().mockImplementation(() =>
Promise.reject({
response: {
status: 404,
text: () => 'not found',
},
})
);
const fakeListIndexes: () => Promise<string[]> = jest
.fn()
.mockImplementation(() => Promise.resolve(['foo', 'bar']));
const IOA = {
configureIndex: fakeConfigure,
listIndexes: fakeListIndexes,
} as IndexOperationsApi;

jest.mock('../../pinecone-generated-ts-fetch', () => ({
IndexOperationsApi: IOA,
}));

const toThrow = async () => {
await configureIndex(IOA)('index-name', { replicas: 10 });
};

await expect(toThrow).rejects.toThrow(PineconeNotFoundError);
await expect(toThrow).rejects.toThrow(
"Index 'index-name' does not exist. Valid index names: ['foo', 'bar']"
);
});
});
});
71 changes: 1 addition & 70 deletions src/control/__tests__/createCollection.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import { createCollection } from '../createCollection';
import {
PineconeArgumentError,
PineconeBadRequestError,
PineconeInternalServerError,
PineconeNotFoundError,
} from '../../errors';
import { PineconeArgumentError } from '../../errors';
import { IndexOperationsApi } from '../../pinecone-generated-ts-fetch';
import type { CreateCollectionOperationRequest as CCOR } from '../../pinecone-generated-ts-fetch';

Expand Down Expand Up @@ -149,68 +144,4 @@ describe('createCollection', () => {
},
});
});

describe('http error mapping', () => {
test('when 500 occurs', async () => {
const IOA = setOpenAPIResponse(() =>
Promise.reject({
response: {
status: 500,
text: () => 'backend error message',
},
})
);

const toThrow = async () => {
await createCollection(IOA)({
name: 'collection-name',
source: 'index-name',
});
};

await expect(toThrow).rejects.toThrow(PineconeInternalServerError);
});

test('when 400 occurs, displays server message', async () => {
const IOA = setOpenAPIResponse(() =>
Promise.reject({
response: {
status: 400,
text: () => 'backend error message',
},
})
);
const toThrow = async () => {
await createCollection(IOA)({
name: 'collection-name',
source: 'index-name',
});
};

await expect(toThrow).rejects.toThrow(PineconeBadRequestError);
await expect(toThrow).rejects.toThrow('backend error message');
});

test('when 404 occurs, show available indexes', async () => {
const IOA = setOpenAPIResponse(() =>
Promise.reject({
response: {
status: 404,
text: () => 'not found',
},
})
);
const toThrow = async () => {
await createCollection(IOA)({
name: 'collection-name',
source: 'index-name',
});
};

await expect(toThrow).rejects.toThrow(PineconeNotFoundError);
await expect(toThrow).rejects.toThrow(
"Index 'index-name' does not exist. Valid index names: ['foo', 'bar']"
);
});
});
});
56 changes: 1 addition & 55 deletions src/control/__tests__/createIndex.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import { createIndex } from '../createIndex';
import {
PineconeBadRequestError,
PineconeConflictError,
PineconeInternalServerError,
} from '../../errors';
import { IndexOperationsApi } from '../../pinecone-generated-ts-fetch';
import type {
CreateIndexRequest,
Expand All @@ -23,7 +18,7 @@ const setupCreateIndexResponse = (
.mockImplementation(() =>
isCreateIndexSuccess
? Promise.resolve(createIndexResponse)
: Promise.reject({ response: createIndexResponse })
: Promise.reject(createIndexResponse)
);

// unfold describeIndexResponse
Expand Down Expand Up @@ -134,53 +129,4 @@ describe('createIndex', () => {
});
});
});

describe('http error mapping', () => {
test('when 500 occurs', async () => {
const IOA = setupCreateIndexResponse(
{ status: 500, text: () => 'backend error message' },
undefined,
false
);

const toThrow = async () => {
const createIndexFn = createIndex(IOA);
await createIndexFn({ name: 'index-name', dimension: 10 });
};

await expect(toThrow).rejects.toThrow(PineconeInternalServerError);
});

test('when 400 occurs, displays server message', async () => {
const serverError = 'there has been a server error!';
const IOA = setupCreateIndexResponse(
{ status: 400, text: () => serverError },
undefined,
false
);

const toThrow = async () => {
const createIndexFn = createIndex(IOA);
await createIndexFn({ name: 'index-name', dimension: 10 });
};

await expect(toThrow).rejects.toThrow(PineconeBadRequestError);
await expect(toThrow).rejects.toThrow(serverError);
});

test('when 409 occurs', async () => {
const IOA = setupCreateIndexResponse(
{ status: 409, text: () => 'conflict error message' },
undefined,
false
);

const toThrow = async () => {
const createIndexFn = createIndex(IOA);
await createIndexFn({ name: 'index-name', dimension: 10 });
};

await expect(toThrow).rejects.toThrow(PineconeConflictError);
});
});
});
73 changes: 1 addition & 72 deletions src/control/__tests__/deleteCollection.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
import { deleteCollection } from '../deleteCollection';
import {
PineconeArgumentError,
PineconeInternalServerError,
PineconeNotFoundError,
} from '../../errors';
import { PineconeArgumentError } from '../../errors';
import { IndexOperationsApi } from '../../pinecone-generated-ts-fetch';
import type { DeleteCollectionRequest as DCR } from '../../pinecone-generated-ts-fetch';

Expand Down Expand Up @@ -59,71 +55,4 @@ describe('deleteCollection', () => {
);
});
});

describe('uses http error mapper', () => {
test('it should map errors with the http error mapper (500)', async () => {
const IOA = setupMocks(() =>
Promise.reject({ response: { status: 500, text: async () => '' } })
);

// @ts-ignore
const expectToThrow = async () =>
await deleteCollection(IOA)('collection-name');

expect(expectToThrow).rejects.toThrowError(PineconeInternalServerError);
});
});

describe('custom error mapping', () => {
test('not found (404), fetches and shows available collection names', async () => {
const IOA = setupMocks(
() =>
Promise.reject({ response: { status: 404, text: async () => '' } }),
() => Promise.resolve(['foo', 'bar'])
);

// @ts-ignore
const expectToThrow = async () =>
await deleteCollection(IOA)('collection-name');

expect(expectToThrow).rejects.toThrowError(PineconeNotFoundError);
expect(expectToThrow).rejects.toThrowError(
`Collection 'collection-name' does not exist. Valid collection names: ['foo', 'bar']`
);
});

test('not found (404), fetches and shows available collection names (empty list)', async () => {
const IOA = setupMocks(
() =>
Promise.reject({ response: { status: 404, text: async () => '' } }),
() => Promise.resolve([])
);

// @ts-ignore
const expectToThrow = async () =>
await deleteCollection(IOA)('collection-name');

expect(expectToThrow).rejects.toThrowError(PineconeNotFoundError);
expect(expectToThrow).rejects.toThrowError(
`Collection 'collection-name' does not exist. Valid collection names: []`
);
});

test('not found (404), error while fetching collection list', async () => {
const IOA = setupMocks(
() =>
Promise.reject({ response: { status: 404, text: async () => '' } }),
() => Promise.reject('error')
);

// @ts-ignore
const expectToThrow = async () =>
await deleteCollection(IOA)('collection-name');

expect(expectToThrow).rejects.toThrowError(PineconeNotFoundError);
expect(expectToThrow).rejects.toThrowError(
`Collection 'collection-name' does not exist.`
);
});
});
});
Loading