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

Preserve the original error name instead of returning raw AbortError #57550

Merged
merged 4 commits into from
Feb 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface IHttpFetchError extends Error
| Property | Type | Description |
| --- | --- | --- |
| [body](./kibana-plugin-public.ihttpfetcherror.body.md) | <code>any</code> | |
| [name](./kibana-plugin-public.ihttpfetcherror.name.md) | <code>string</code> | |
| [req](./kibana-plugin-public.ihttpfetcherror.req.md) | <code>Request</code> | |
| [request](./kibana-plugin-public.ihttpfetcherror.request.md) | <code>Request</code> | |
| [res](./kibana-plugin-public.ihttpfetcherror.res.md) | <code>Response</code> | |
Expand Down
42 changes: 37 additions & 5 deletions src/core/public/http/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe('Fetch', () => {
});
afterEach(() => {
fetchMock.restore();
fetchInstance.removeAllInterceptors();
});

describe('http requests', () => {
Expand Down Expand Up @@ -287,6 +288,42 @@ describe('Fetch', () => {
});
});

it('preserves the name of the original error', async () => {
expect.assertions(1);

const abortError = new DOMException('The operation was aborted.', 'AbortError');

fetchMock.get('*', Promise.reject(abortError));

await fetchInstance.fetch('/my/path').catch(e => {
expect(e.name).toEqual('AbortError');
});
});

it('exposes the request to the interceptors in case of aborted request', async () => {
const responseErrorSpy = jest.fn();
const abortError = new DOMException('The operation was aborted.', 'AbortError');

fetchMock.get('*', Promise.reject(abortError));

fetchInstance.intercept({
responseError: responseErrorSpy,
});

await expect(fetchInstance.fetch('/my/path')).rejects.toThrow();

expect(responseErrorSpy).toHaveBeenCalledTimes(1);
const interceptedResponse = responseErrorSpy.mock.calls[0][0];

expect(interceptedResponse.request).toEqual(
expect.objectContaining({
method: 'GET',
url: 'http://localhost/myBase/my/path',
})
);
expect(interceptedResponse.error.name).toEqual('AbortError');
});

it('should support get() helper', async () => {
fetchMock.get('*', {});
await fetchInstance.get('/my/path', { method: 'POST' });
Expand Down Expand Up @@ -368,11 +405,6 @@ describe('Fetch', () => {
fetchMock.get('*', { foo: 'bar' });
});

afterEach(() => {
fetchMock.restore();
fetchInstance.removeAllInterceptors();
});

it('should make request and receive response', async () => {
fetchInstance.intercept({});

Expand Down
10 changes: 3 additions & 7 deletions src/core/public/http/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,7 @@ export class Fetch {
try {
response = await window.fetch(request);
} catch (err) {
if (err.name === 'AbortError') {
throw err;
} else {
throw new HttpFetchError(err.message, request);
}
throw new HttpFetchError(err.message, err.name ?? 'Error', request);
}

const contentType = response.headers.get('Content-Type') || '';
Expand All @@ -170,11 +166,11 @@ export class Fetch {
}
}
} catch (err) {
throw new HttpFetchError(err.message, request, response, body);
throw new HttpFetchError(err.message, err.name ?? 'Error', request, response, body);
}

if (!response.ok) {
throw new HttpFetchError(response.statusText, request, response, body);
throw new HttpFetchError(response.statusText, 'Error', request, response, body);
}

return { fetchOptions, request, response, body };
Expand Down
3 changes: 3 additions & 0 deletions src/core/public/http/http_fetch_error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,19 @@ import { IHttpFetchError } from './types';

/** @internal */
export class HttpFetchError extends Error implements IHttpFetchError {
public readonly name: string;
public readonly req: Request;
public readonly res?: Response;

constructor(
message: string,
name: string,
public readonly request: Request,
public readonly response?: Response,
public readonly body?: any
) {
super(message);
this.name = name;
this.req = request;
this.res = response;

Expand Down
1 change: 1 addition & 0 deletions src/core/public/http/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ export interface IHttpResponseInterceptorOverrides<TResponseBody = any> {

/** @public */
export interface IHttpFetchError extends Error {
readonly name: string;
readonly request: Request;
readonly response?: Response;
/**
Expand Down
2 changes: 2 additions & 0 deletions src/core/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,8 @@ export type IContextProvider<THandler extends HandlerFunction<any>, TContextName
export interface IHttpFetchError extends Error {
// (undocumented)
readonly body?: any;
// (undocumented)
readonly name: string;
// @deprecated (undocumented)
readonly req: Request;
// (undocumented)
Expand Down