-1

This function returns a Promise with an interface that contains two objects, within that function I have a fetch request where I return that interface.

The interface contains two objects called data and default, where data is the fetch response either json(), arraybuffer(), etc. default is the complete object of the fetch Response.

Everything goes in order when the Promise is resolved when the response is ok. But in addition, there is a property that I receive in the function that indicates that the function has to be executed again in case there is a 401 error (or any other outside of ok), this property is of type Array which can receive status codes where the request needs to be retried.

export function request(url: RequestInfo, options: IOptions): Promise<IResponse> {
// more code;
  var defaults;
  var response: IResponse;
  return new Promise<IResponse>((resolve, reject) => {
    fetch(url, {
      method: options?.method ?? "get",
      headers: headersInit,
      body: options.data,
    })
      .then((resp) => {
        defaults = resp;
        if (resp.ok) {
          if (utils.IsNullOrUndefined(options.result)) {
            return resp.json();
          } else {
            switch (options.result) {
              case "json":
                return resp.json();
                break;
              case "arrayBuffer":
                return resp.arrayBuffer();
                break;
              case "blob":
                return resp.blob();
                break;
              case "text":
                return resp.text();
                break;
            }
          }
        } else {          
          if(options.retryOn){                        
            if(Array.isArray(options.retryOn) && options.retryOn.indexOf(resp.status) !== -1){
              // Here I try to execute the function in case retryOn contains the status code that returned fetch
              response = null;
              defaults = null;
              return request(url, options);
            }
          }
          else {          
            response = {
              data: null,
              default: defaults,
            };
            reject(response);
          }
        }
      })
      .then((res) => {
        response = {
          data: res,
          default: defaults,
        };
        resolve(response);
      });
  });
}

When the request returns an ok code, the Promise has the following fields as objects:

enter image description here But if there was a status code other than ok, and in retryOn the required status code is found, the function is executed again and the following objects return. enter image description here

The data object is duplicated

I run the function like this:

request("URL", {
     method: "get",
     retryOn: [401]
}).then(response => {
      console.log(response.data);
      console.log(response.default);
}).catch(error => {
      console.log(error);
})
Dominik
  • 6,078
  • 8
  • 37
  • 61
Isaías Orozco Toledo
  • 1,919
  • 5
  • 19
  • 35
  • 3
    don't repeatedly create promises in a recursive function. – Randy Casburn Oct 14 '20 at 21:00
  • 1
    It's the line `response = { data: res, default: defaults, };` If `res` already has a `data` property, the code will add another on top. Either follow @Randy's advice, or check: `response = res.data ? res : { data: res, default: defaults };` – Heretic Monkey Oct 14 '20 at 21:03
  • [Don't do this](https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it). – Jared Smith Oct 14 '20 at 21:35

1 Answers1

1

Your entire function can be re-written like so:

async function request2(url, options): Promise<IResponse> {
  const resp = await fetch(url, {
    method: options?.method ?? "get",
    headers: headersInit,
    body: options.data,
  });
  let result;
  if (resp.ok) {
    result = await (resp[options.result] || resp.json)();
  } else if (options.retryOn && options.retryOn.includes(resp.status)) {
    // NOTE: potentially infinite loop. You won't notice much in the UI
    // but don't try this on cellular data unless you hate your customers.
    return await request2(url, options);
  } else {
    // I've changed this, you shouldn't reject a Promise with a non-error.
    const text = await resp.text();
    throw new Error(text);
  }

  return {data: result, defaults: resp};
};

The key here is that the retry will return so it doesn't get wrapped up in the data format at the last statement, where as your current logic doesn't really have a way to handle this nicely with non async notation.

This has a lot of nice properties like not having nested conditionals. Or asynchronously altered closed-over vars.

Tadhg McDonald-Jensen
  • 20,699
  • 5
  • 35
  • 59
Jared Smith
  • 19,721
  • 5
  • 45
  • 83
  • your chaining logic is incorrect, it ends with an unconditional `throw` so that version of the function always returns a rejected promise. You probably want something like this instead: `.then(SUCC_HANDLE, (err)=>resp.text().then(text=>{throw new Error(text)}))` nesting a `.then` inside the callback for an error is ugly but the correct way to do an additional await in a separate error handling chain. – Tadhg McDonald-Jensen Oct 14 '20 at 22:22
  • no sorry, the `throw new Error("error")` should just be replaced with `return resp.text().then(text=>{throw new Error(text)})` then remove a lot of the weird extra thens to try to get error handling correct. – Tadhg McDonald-Jensen Oct 14 '20 at 22:26
  • 1
    @TadhgMcDonald-Jensen you are correct I deleted that portion. I'm too lazy to work this out with raw Promises, and you're correct that it's going to rely on stuffing things in a closure variable or some other hack. If you want to edit it in feel free or write an answer with the details, I unfortunately have to get back to work. – Jared Smith Oct 14 '20 at 22:26