1

I wrote a function to catch non-200 results with fetch:

 1 function $get(url, callback) {
 2  fetch(url, {credentials: "same-origin"})
 3    .then(resp => {
 4      if (!resp.ok) {
 5        resp.text().then((mesg) => {
 6          throw {"stat": resp.status, "mesg": mesg.trim()}
 7        })
 8        return resp.text()
 9      } 
10      return resp.json() 
11    })
12    .then(data => callback({"stat": 200, "data": data}))
13    .catch(error => callback(error))
14}

I got error on line 9:

ERROR: TypeError: Failed to execute 'text' on 'Response': body stream already read

The reason that I have to write code shown in line 5~7 is that if I wrote:

if (!resp.ok) {
  throw {"stat": resp.status, "mesg": resp.statusText}
return resp.json()

I will get error message like {"stat": 403, "mesg": "Forbidden"}, while what I want is: {"stat": 403, "mesg": "invalid user name or password"}.

On the server side my go program will generate non-200 reply like this:

> GET /api/login?u=asdf&p=asdf HTTP/1.1
> Host: localhost:7887
> User-Agent: curl/7.68.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 403 Forbidden
< Content-Type: text/plain; charset=utf-8
< X-Content-Type-Options: nosniff
< Date: Sat, 17 Jul 2021 11:53:16 GMT
< Content-Length: 25
< 
invalid username or password

I.e. the go library do not modify http status text, instead, put error message in body, which maybe mandated by the http standard (e.g. status text cannot be changed).

So, my question is, either:

  • How to read the body of non-200 reply without using promise?
  • or, how to reply an "empty" promise in case of error, to prevent the stream being read again?

=== EDIT ===

The following code works OK, however, it seems to use "anti" pattern as pointed out by comments:

function $get(url, callback) {
  fetch(url, {credentials: "same-origin"})
    .then(resp => {
      if (!resp.ok) {
        resp.text().then((mesg) => {
          callback({"stat": resp.status, "mesg": mesg.trim()})
        })
        return new Promise(function(_, _) {}) 
      } 
      return resp.json()
    })
    .then(data => callback({"stat": 200, "data": data}))
    .catch(error => { console.log(`GET ${url}\nERROR: ${error}`) })
}

However, this doe not work:

function $get(url, callback) {
  fetch(url, {credentials: "same-origin"})
    .then(resp => {
      if (!resp.ok) {
        resp.text().then((mesg) => {
          throw `{"stat": resp.status, "mesg": mesg.trim()}`
        }) 
      } 
      return resp.json()
    })
    .then(data => callback({"stat": 200, "data": data}))
    .catch(error => { console.log(`GET ${url}\nERROR: ${error}`) })
}

The throw will generate this error, instead of passing control to the catch below:

127.0.0.1/:1 Uncaught (in promise) {"stat": resp.status, "mesg": mesg.trim()}
xrfang
  • 1,754
  • 4
  • 18
  • 36
  • 4
    Don't mix promises with a callback system. It is an antipattern. – trincot Jul 17 '21 at 12:39
  • @trincot, so, my purpose is to let my code know what is the http status code as well as get its reponse body, without callbacks what is the "pro" pattern way? – xrfang Jul 17 '21 at 12:51
  • The caller should use the promise object that your function should *return*... so you need to do `return fetch(......`. Then *that* returned promise has a `then` method with a callback that can be passed to it... or use `await` syntax. – trincot Jul 17 '21 at 13:04
  • Re your edit: "*The following code works OK*" - no it doesn't, it sometimes calls `callback` twice. "*However, this doe not work:*" - yes, you're missing the `return` keyword in front of the `resp.text().then(…)` promise chain. Re-read Poul's answer please. – Bergi Jul 17 '21 at 13:07
  • @Bergi: I am a bit confused, and yes I tried Poul's code, and got error last time, and I tried again, it worked.... – xrfang Jul 17 '21 at 13:14
  • BTW, you are allowed to change the HTTP status text, nothing keeps you from doing that. If you want to reply `401 invalid username or password` instead of `401 Unauthorized`, you absolutely can. – Tomalak Jul 17 '21 at 13:37
  • @Tomalak it's golang stdlib's behavior to keep standard status text, and put custom text in body – xrfang Jul 17 '21 at 13:41
  • Ahh, I see. Well, let's say nothing in the HTTP spec mandates a certain wording for the status text. I've read your question a couple of times now, I'm still unclear about the desired behavior. Imagine a synchronous JS function `get(url)`, what should be the return values for e.g. `200`, `401`, `500` and for network errors? – Tomalak Jul 17 '21 at 13:49
  • @Tomalak expected: {"stat": 200, "data": }, {"stat": xxx, "mesg": } are to be sent to callback/downstream, and global handler for network errors – xrfang Jul 17 '21 at 13:53

2 Answers2

3

Considering you are using fetch you can also use async/await and do the following. :

async function $get(url, callback) {
  try {
    const resp = await fetch(url, {credentials: "same-origin"});
    
    if (!resp.ok) {
      // this will end up in the catch statement below
      throw({ stat: resp.status, mesg: (await resp.text()).trim());
    }
    
    callback({ stat: 200, data: await resp.json() });
  } catch(error) {
    callback(error);
  }
}

I don't understand why you would use callback functions though :) those are so 1999


To explain your error, you are calling resp.text() twice when there is an error. To prevent that, you should immediately return the promise chained from the first resp.text() call. This will also throw the error and end up in the catch block without reaching the consecutive then() statement:

function $get(url, callback) {
 fetch(url, {credentials: "same-origin"})
   .then(resp => {
     if (!resp.ok) {
       return resp.text().then((mesg) => {
//     ^^^^^^
         throw {stat: resp.status, error: mesg.trim()}
       });
     } 
     return resp.json() ;
   })
   .then(data => callback({stat: 200, data }))
   .catch(error => callback(error))
}

A "proper" $get function which doesn't use callbacks:

function $get(url) {
  return fetch(url, {credentials: "same-origin"}).then(async (resp) => {
    const stat = resp.status;

    if (!resp.ok) {
      throw({ stat, error: (await resp.text()).trim() });
    }

    return { stat, data: await resp.json() };
  });
}

Which you can consume like:

$get('https://example.com')
  .then(({ stat, data }) => {

  })
  .catch({ stat, error}) => {

  })
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
Poul Kruijt
  • 69,713
  • 12
  • 145
  • 149
  • The first part of your answer **maybe** a good idea, but I haven't try it yet, because I found that by simply doing `return new Promise(function(_, _) {}) ` will eliminate the error. The second part is **wrong**, I tried in chrome and god `not found: undefined` when handling wrong password – xrfang Jul 17 '21 at 12:41
  • 2
    @xrfang That is an anti-pattern. You just need to `return` the first `resp.text()` promise as shown in second part of this answer. Then when you throw the error is gets passed down the chain – charlietfl Jul 17 '21 at 12:42
  • 1
    @xrfang don't create a new promise when you are already working with promises. That will clutter your code even more and will result in a `then` chain of hell. Same with the callback methods. To keep the code clean you are way better off using `async/await`, – Poul Kruijt Jul 17 '21 at 12:42
  • @PoulKruijt sorry, your second part is not a solution just explain why I am wrong. Before trying the async/wait stuff, I have to explain that if I simply return `resp.text()` then in the callback, I will not be able to tell if there is an error or not, because I lost the http status code. – xrfang Jul 17 '21 at 12:47
  • @PoulKruijt if callbacks is so ancient like in 1999, what is your solution then? Although you proposed await, it still use callbacks? – xrfang Jul 17 '21 at 12:49
  • 1
    @xrfang Using async/await is not required. In your code just change the `return` and get rid of the second `resp.text()`. You will just return the first one that throws the error in `then()` instead and be able to `catch()` it further down the chain – charlietfl Jul 17 '21 at 12:53
  • Your `async function $get()` can be collapsed into `const $get = (url) => fetch(url, {credentials: "same-origin"});` at no loss of functionality. And I completely agree, the `callback` needs to be dropped. – Tomalak Jul 17 '21 at 12:58
  • @xrfang I've updated my code with a `$get` function without callbacks. About your other question. Did you try it? The `throw` statement contains the `stat` which has the response status. This is thrown to the `catch` statement and send to the `callback` function – Poul Kruijt Jul 17 '21 at 12:59
  • @Tomalak I accidentally pressed `ctrl+enter` while typing, thus prematurely submitting my updated answer :D. And some people like to use the `function` keyword instead of `const` for functions – Poul Kruijt Jul 17 '21 at 13:00
  • @PoulKruijt @Tomalak as a matter of fact, I am a backend developer and only use JS as hobby... I totally agree with you that I should avoid callback if the JS world is shifting away from it. However, please see my edit in the main question, ignoring pattern problems in the code, what if my purpose is to ONLY catch network errors like fetch is suposed to do, and consume all returns, including non-200 in the `then()` block? so that I can use `switch(stat) {}` to process it? – xrfang Jul 17 '21 at 13:09
  • @xrfang check my 2nd code block, you -have- to return the `resp.text().then(` statement – Poul Kruijt Jul 17 '21 at 13:11
  • @xrfang *"avoid callback if the JS world is shifting away from it"* that's not even the point. Callbacks are fine. Just don't mix promise-based code and callback-based code. Promise-based implementations are becoming more common, but fundamentally, promises are nothing but an elaborate method for callback registration. `fetch` already does what you want by default: *"The Promise returned from fetch() won’t reject on HTTP error status even if the response is an HTTP 404 or 500."* (see [MDN](https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch)). – Tomalak Jul 17 '21 at 13:20
  • @Tomalak @PoulKruijt I succeeded with your latest version of `$get`, just one more question, as of 2021, is there any update to the answer of this question: https://stackoverflow.com/questions/31472439/catch-all-unhandled-javascript-promise-rejections Note: I am using "modern" browsers, **not** node.js. – xrfang Jul 17 '21 at 13:48
1

Just return the rejected promise and you are good to go

 1 function $get(url, callback) {
 2  fetch(url, {credentials: "same-origin"})
 3    .then(resp => {
 4      if (!resp.ok) {
 5        return resp.text().then((mesg) => {
 6          throw {"stat": resp.status, "mesg": mesg.trim()}
 7        })
 9      } 
10      return resp.json() 
11    })
12    .then(data => callback({"stat": 200, "data": data}))
13    .catch(error => callback(error))
14}
Mechanic
  • 5,015
  • 4
  • 15
  • 38