CODEWITHSUNDEEP
node.jsreactjs
Ryan Vice
Ryan Vice

Reputation: 2181

How to return a rejected promise from a function that wants to do some async processing first

I have this wrapper around fetch that we use and in it I want to allow the consumer to call it like shown in this test.

  it("When we get a failure response Then we get rejected promise with error", done => {
    try {
      fetch.mockImplementation(
        mockFetch([
          {
            url: "/url",
            errorResponse: true,
            response: () => ({ error: "whoops" })
          }
        ])
      );
      store.getState.mockReturnValue({
        userContext: {}
      });

      const p = http.get("url", { sampleConfig: "sample" });

      setTimeout(() => {
        p.then(r => expect(r.result)).catch(e => {  
          try {
            expect(e.error).toBe("whoops");  <== CODE MAKES IT HERE
            done();
          } catch (e) {
            done.fail(e);
          }
        });
      });
    } catch (e) {
      done.fail(e);
    }
  });

This is the code that is called by http.get

  return fetch(buildUrl(url), addJwtToken(config)).then(response => {
    if (response.headers) {
      setJwtTokenFromHeaderResponse(response.headers.get("Authorization"));
    }

    if (response.ok) {
      if (response.headers.map["content-type"].includes("stream")) {
        return response;
      }
      return response.json();
    }

    const unauthorized = 401;
    if (response.status === unauthorized) {
      // All else failed so redirect user to FMS to reauthenticate
      localStorage.removeItem("jwtToken");
      return response.json().then(() => redirectToSignOut());
    }

    return response.json().then(r => Promise.reject(r));   <== THIS RIGHT HERE
  });

I don't want the caller to have to deal with doing

http.get('/foo').catch(r => r.json()).then(e => processError(e))

And instead want to handle resolving the json() error for them so they can just do this.

http.get('/foo').catch(processError)

I feel it's a reasonable design but maybe I'm missing something as node complains in my test by showing these warnings.

(node:6412) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): [object Promise]
(node:6412) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
(node:6412) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 2)

I can't figure out how to get it to stop spamming my tests with these warning or to figure out if there is an idiomatic way to accomplish my goals.

Edit 1 I updated the code to return the promise chain which was an unrelated bug.

Upvotes: 0

Views: 100

Answers (1)

Estus Flask
Estus Flask

Reputation: 222493

There's a number of problems.

There will be an error if a response is processed with response.json() multiple times. response.json() should be called once:

if (response.status === unauthorized) {
  // All else failed so redirect user ot FMS to reauthenticate
  localStorage.removeItem("jwtToken");
  response.json().then(() => redirectToSignOut());
}

return response.json().then(r => Promise.reject(r));

All promises should be chained. This

response.json().then(() => redirectToSignOut());

results in flawed control flow. It's unclear how it's expected to work but every promise should be returned from then.

Using done in tests is an antipattern with promises because its use results in human errors; all modern testing frameworks support promises. try..catch is inappropriate here because it cannot catch asynchronous errors from promises outside async function. setTimeout is inappropriate here, too.

It's also unclear why r.result should be a rejected promise because it isn't defined anywhere in the code.

With async it could be something like:

  it("When we get a failure response Then we get rejected promise with error", async () => {
      fetch.mockImplementation(
        mockFetch([
          {
            url: "/url",
            errorResponse: true,
            response: () => ({ error: "whoops" })
          }
        ])
      );
      store.getState.mockReturnValue({
        userContext: {}
      });

      const responsePromise = http.get("url", { sampleConfig: "sample" })
      await expect(responsePromise).rejects.toEqual({ error: whoops });
  });

Upvotes: 1

Related Questions