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.

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

Improve errors #1656

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
5 changes: 0 additions & 5 deletions jest-disable-built-in-fetch.js

This file was deleted.

3 changes: 2 additions & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ const config = {
'jest-watch-typeahead/filename',
'jest-watch-typeahead/testname',
],
globalSetup: './jest-disable-built-in-fetch.js',
projects: [
{
preset: 'ts-jest',
Expand All @@ -28,6 +27,8 @@ const config = {
'env/',
'token.test.ts',
],
// make sure built-in Node.js fetch doesn't get replaced for consistency
globals: { fetch: global.fetch, AbortController: global.AbortController },
},
{
preset: 'ts-jest',
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@
"@types/jest": "^29.5.11",
"@typescript-eslint/eslint-plugin": "^6.19.0",
"@typescript-eslint/parser": "^6.19.0",
"abort-controller": "^3.0.0",
"brotli-size": "^4.0.0",
"eslint": "^8.56.0",
"eslint-config-prettier": "^9.1.0",
Expand Down
44 changes: 0 additions & 44 deletions src/errors/http-error-handler.ts

This file was deleted.

3 changes: 1 addition & 2 deletions src/errors/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
export * from './http-error-handler';
export * from './meilisearch-api-error';
export * from './meilisearch-communication-error';
export * from './meilisearch-request-error';
export * from './meilisearch-error';
export * from './meilisearch-timeout-error';
export * from './version-hint-message';
36 changes: 13 additions & 23 deletions src/errors/meilisearch-api-error.ts
Original file line number Diff line number Diff line change
@@ -1,30 +1,20 @@
import { MeiliSearchErrorInfo } from '../types';
import { MeiliSearchErrorResponse } from '../types';
import { MeiliSearchError } from './meilisearch-error';

const MeiliSearchApiError = class extends MeiliSearchError {
httpStatus: number;
code: string;
link: string;
type: string;
stack?: string;
export class MeiliSearchApiError extends MeiliSearchError {
override name = 'MeiliSearchApiError';
override cause?: MeiliSearchErrorResponse;
readonly response: Response;

constructor(error: MeiliSearchErrorInfo, status: number) {
super(error.message);
constructor(response: Response, responseBody?: MeiliSearchErrorResponse) {
super(
responseBody?.message ?? `${response.status}: ${response.statusText}`,
);

// Make errors comparison possible. ex: error instanceof MeiliSearchApiError.
Object.setPrototypeOf(this, MeiliSearchApiError.prototype);
this.response = response;

this.name = 'MeiliSearchApiError';

this.code = error.code;
this.type = error.type;
this.link = error.link;
this.message = error.message;
this.httpStatus = status;

if (Error.captureStackTrace) {
Error.captureStackTrace(this, MeiliSearchApiError);
if (responseBody !== undefined) {
this.cause = responseBody;
}
}
};
export { MeiliSearchApiError };
}
17 changes: 4 additions & 13 deletions src/errors/meilisearch-error.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,7 @@
class MeiliSearchError extends Error {
constructor(message: string) {
super(message);
export class MeiliSearchError extends Error {
override name = 'MeiliSearchError';

// Make errors comparison possible. ex: error instanceof MeiliSearchError.
Object.setPrototypeOf(this, MeiliSearchError.prototype);

this.name = 'MeiliSearchError';

if (Error.captureStackTrace) {
Error.captureStackTrace(this, MeiliSearchError);
}
constructor(...params: ConstructorParameters<typeof Error>) {
super(...params);
}
}

export { MeiliSearchError };
9 changes: 9 additions & 0 deletions src/errors/meilisearch-request-error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { MeiliSearchError } from './meilisearch-error';

export class MeiliSearchRequestError extends MeiliSearchError {
override name = 'MeiliSearchRequestError';

constructor(url: string, cause: unknown) {
super(`Request to ${url} has failed`, { cause });
}
}
15 changes: 3 additions & 12 deletions src/errors/meilisearch-timeout-error.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,9 @@
import { MeiliSearchError } from './meilisearch-error';

class MeiliSearchTimeOutError extends MeiliSearchError {
export class MeiliSearchTimeOutError extends MeiliSearchError {
override name = 'MeiliSearchTimeOutError';

constructor(message: string) {
super(message);

// Make errors comparison possible. ex: error instanceof MeiliSearchTimeOutError.
Object.setPrototypeOf(this, MeiliSearchTimeOutError.prototype);

this.name = 'MeiliSearchTimeOutError';

if (Error.captureStackTrace) {
Error.captureStackTrace(this, MeiliSearchTimeOutError);
}
}
}

export { MeiliSearchTimeOutError };
55 changes: 28 additions & 27 deletions src/http-requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { PACKAGE_VERSION } from './package-version';

import {
MeiliSearchError,
httpResponseErrorHandler,
httpErrorHandler,
MeiliSearchApiError,
MeiliSearchRequestError,
} from './errors';

import { addTrailingSlash, addProtocolIfNotPresent } from './utils';
Expand Down Expand Up @@ -143,35 +143,36 @@ class HttpRequests {
}

const headers = { ...this.headers, ...config.headers };
const responsePromise = this.fetchWithTimeout(
constructURL.toString(),
{
...config,
...this.requestConfig,
method,
body,
headers,
},
this.requestTimeout,
);

try {
const result = this.fetchWithTimeout(
constructURL.toString(),
{
...config,
...this.requestConfig,
method,
body,
headers,
},
this.requestTimeout,
);

// When using a custom HTTP client, the response is returned to allow the user to parse/handle it as they see fit
if (this.httpClient) {
return await result;
}
const response = await responsePromise.catch((error: unknown) => {
throw new MeiliSearchRequestError(constructURL.toString(), error);
});

// When using a custom HTTP client, the response is returned to allow the user to parse/handle it as they see fit
if (this.httpClient !== undefined) {
return response;
}

const response = await result.then((res: any) =>
httpResponseErrorHandler(res),
);
const parsedBody = await response.json().catch(() => undefined);
const responseBody = await response.text();
const parsedResponse =
responseBody === '' ? undefined : JSON.parse(responseBody);
Comment on lines +167 to +169
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously if response.json() failed, we swallowed the error. There are cases when the response body is empty, and is parsed to an empty string initially. That is an invalid JSON. This case was handled by simply swallowing the error, now it's handled explicitly, and if for whatever reason Meilisearch returns something non-JSON-valid other than an empty body, it will err, which is what we want, as that's entirely unexpected and should propagate.


return parsedBody;
} catch (e: any) {
const stack = e.stack;
httpErrorHandler(e, stack, constructURL.toString());
if (!response.ok) {
throw new MeiliSearchApiError(response, parsedResponse);
}

return parsedResponse;
}

async fetchWithTimeout(
Expand Down
9 changes: 3 additions & 6 deletions src/indexes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import {
MeiliSearchError,
MeiliSearchCommunicationError,
MeiliSearchRequestError,
versionErrorHintMessage,
MeiliSearchApiError,
} from './errors';
Expand Down Expand Up @@ -360,7 +360,7 @@ class Index<T extends Record<string, any> = Record<string, any>> {
Promise<ResourceResults<D[]>>
>(url, parameters);
} catch (e) {
if (e instanceof MeiliSearchCommunicationError) {
if (e instanceof MeiliSearchRequestError) {
e.message = versionErrorHintMessage(e.message, 'getDocuments');
} else if (e instanceof MeiliSearchApiError) {
e.message = versionErrorHintMessage(e.message, 'getDocuments');
Expand Down Expand Up @@ -585,10 +585,7 @@ class Index<T extends Record<string, any> = Record<string, any>> {

return new EnqueuedTask(task);
} catch (e) {
if (
e instanceof MeiliSearchCommunicationError &&
isDocumentsDeletionQuery
) {
if (e instanceof MeiliSearchRequestError && isDocumentsDeletionQuery) {
e.message = versionErrorHintMessage(e.message, 'deleteDocuments');
} else if (e instanceof MeiliSearchApiError) {
e.message = versionErrorHintMessage(e.message, 'deleteDocuments');
Expand Down
11 changes: 7 additions & 4 deletions src/types/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ export type TaskObject = Omit<EnqueuedTaskObject, 'taskUid'> & {
// Query parameters used to filter the tasks
originalFilter?: string;
};
error: MeiliSearchErrorInfo | null;
error: MeiliSearchErrorResponse | null;
duration: string;
startedAt: string;
finishedAt: string;
Expand Down Expand Up @@ -655,13 +655,16 @@ export interface FetchError extends Error {
code: string;
}

export type MeiliSearchErrorInfo = {
code: string;
link: string;
export type MeiliSearchErrorResponse = {
message: string;
// @TODO: Could be typed, but will it be kept updated? https://www.meilisearch.com/docs/reference/errors/error_codes
code: string;
// @TODO: Could be typed https://www.meilisearch.com/docs/reference/errors/overview#errors
type: string;
link: string;
};

// @TODO: This doesn't seem to be up to date, and its usefullness comes into question.
export const ErrorStatusCode = {
/** @see https://www.meilisearch.com/docs/reference/errors/error_codes#index_creation_failed */
INDEX_CREATION_FAILED: 'index_creation_failed',
Expand Down
Loading