fetch exception should be rethrowed in kfetch's promise

[spacedragon]

Describe the bug:

It is possible that fetch throw an exception:
I think this line should be wrapped in a try-catch block and re-throw using the reject method.

kibana/src/ui/public/kfetch/kfetch.ts

Line 70 in b434652

const res = await fetch(fullUrl, combinedFetchOptions);

Steps to reproduce:

const abortController = new AbortController()
kfetch({
   pathname: .....,
   signal: abortController.signal
})
abortController.abort()

Expected behavior:

Errors in browser console (if relevant):

Any additional context:
In CodeSearch project, we are trying to attach a AbortController signal to let user be able to cancel a fetch. When a fetch is aborted, it throws a AbortError.
See here:
https://developer.mozilla.org/en-US/docs/Web/API/AbortController/abort

[epixa]

@elastic/kibana-platform

And @spalger in particular since this seems to be impacting code search

[spalger]

@sqren any idea how this might impact the work to add interceptors to kfetch?

[sorenlouv]

@spalger not 100%. First need to be sure I understand the question completely.
I'm probably missing something but if the problem is that fetch throws, can't you then catch it like:

const abortController = new AbortController()
kfetch({
   pathname: .....,
   signal: abortController.signal
}).catch(...)
abortController.abort()

I think this line should be wrapped in a try-catch block and re-throw using the reject method.

How is that different from now? If fetch throws it'll be turned into a rejected promise.
These two are the same:

new Promise((resolve, reject) => {
    throw new Error('throwing')
})

new Promise((resolve, reject) => {
    reject(new Error('rejecting'))
})

Again, sorry if I'm missing something. Please let me know! :)

Btw: there is also kfetchAbortable which is a little nicer to work with.

[sorenlouv]

Ping @spacedragon

[spacedragon]

Here are some codes from codesearch, I added 2 console.log lines.

public async sendRequest(
    method: string,
    params: any,
    signal?: AbortSignal
  ): Promise<ResponseMessage> {
    try {
      console.log('invoking kfetch');
      const response = await kfetch({
        pathname: `${this.baseUri}/${method}`,
        method: 'POST',
        body: JSON.stringify(params),
        signal,
      });
      return response as ResponseMessage;
    } catch (e) {
      console.log('error from kfetch', e);
      const error = e.body;
      throw new ResponseError<any>(error.code, error.message, error.data);
    }
  }

I thought "error from kfetch" would be printed out, but it didn't. Our code couldn't catch the AbortError.

[spacedragon]

Here are some codes from kfetch, I'm adding some comments :

const fetching = new Promise<any>(async (resolve, reject) => {
    // I think problem happens here,  when an AbortController cancel a fetch request
   // a AbortError throws from the following line
    const res = await fetch(fullUrl, combinedFetchOptions);
   // thus no response is returned,  code won't reach here.
    if (res.ok) {
      return resolve(await res.json());
    }

    ...

  // this promise won't catch any AbortError because the Exception throws before here.
  return fetching;

I think this might fix the problem:

const fetching = new Promise<any>(async (resolve, reject) => {
   let res;
    try {
        res = await fetch(fullUrl, combinedFetchOptions);
    } catch(error) {
        reject(error)
    }
    if (res && res.ok) {
      return resolve(await res.json());
    }
...

[spacedragon]

I'm not sure whether my previous comment is true. I read the doc from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/promise#Syntax.
It says

If an error is thrown in the executor function, the promise is rejected. The return value of the executor is ignored.

If Promise works this way, why can't I catch the error?

[sorenlouv]

@spacedragon
Sorry, you are right! We pass an async function to the Promise constructor, which is causing the issue. This PR is about to be merged, and should fix your problem. Please try it out and let me know how it goes:

git checkout -b sqren-add-kfetch-interceptor master
git pull https://github.com/sqren/kibana.git add-kfetch-interceptor