Core HTTP fetch returns incorrect error when a request is aborted

[lukasolson]

Steps to reproduce:

const abortController = new AbortController();
const { signal } = abortController;
const promise = core.http.fetch({
  path: '/path/to/some/route',
  method: 'GET',
  signal
});
setTimeout(() => abortController.abort(), 5);
await promise;

You'd expect the error to be thrown here to be a DOM AbortError, but instead you get TypeError: Cannot read property 'credentials' of undefined.

It looks like we're not properly checking if request exists in the unauthorized response HTTP interceptor. This probably happened here, since we were no longer throwing our own error, but forwarding along the error from the DOM.

[elasticmachine]

Pinging @elastic/kibana-platform (Team:Platform)

[elasticmachine]

Pinging @elastic/kibana-security (Team:Security)

[kobelb]

While it is possible to add a check to

kibana/x-pack/plugins/security/public/session/unauthorized_response_http_interceptor.ts

Line 29 in 6826be2

if (httpErrorResponse.request.credentials === 'omit') {

to ensure the request exists before checking request.credentials === 'omit', I'm hesitant to do so. If there is a "response error", a "request" conceptually has to exist and the types for the HttpInterceptorResponseError have the error always being available.

[joshdover]

I agree with @kobelb, this appears to be a bug in the Core HttpService. The types are not matching the runtime results. We'll prioritize fixing this on the Platform team.

[pgayvallet]

kibana/src/core/public/http/fetch.ts

Lines 146 to 154 in 1bb59af

	try {
	response = await window.fetch(request);
	} catch (err) {
	if (err.name === 'AbortError') {
	throw err;
	} else {
	throw new HttpFetchError(err.message, request);
	}
	}

If I remember correctly, this was done to avoid loosing the AbortError type when a request is canceled, as it allowed to check this way if the request was aborted from the calling code. Not sure how to keep this behavior if we don't return the original error in that case.

• Maybe constructing an error and setting it's name to AbortError is enough?
• Or maybe we consolidate the original AbortError by injecting the request to it?

[lukasolson]

If I remember correctly, this was done to avoid loosing the AbortError type when a request is canceled, as it allowed to check this way if the request was aborted from the calling code. Not sure how to keep this behavior if we don't return the original error in that case.

Yes, that's correct.

Maybe constructing an error and setting it's name to AbortError is enough?
Or maybe we consolidate the original AbortError by injecting the request to it?

I think either of these approaches would work for our purposes in handling aborted requests.

[pgayvallet]

From a quick inspection, every check agains AbortError in the codebase are done with

if (error.name === 'AbortError') {...}

For consistency, adding a name attribute to our HttpFetchError and IHttpFetchError types is probably the best.