bent icon indicating copy to clipboard operation
bent copied to clipboard

status 201 should not throw error by default

Open hst-m opened this issue 4 years ago • 8 comments

201 status code:

The request has succeeded and a new resource has been created as a result. This is typically the response sent after POST requests, or some PUT requests.

This is a successful status response and is commonly used for post request responses, it should not throw error by default

hst-m avatar Nov 29 '20 22:11 hst-m

I face the same issue. Though this is easy to handle, just add 201 to statusCodes.

But I rather expect to see 201 or other codes to be returned in the response/resultant object. Predefining acceptable codes would lead to obscured error handling.

bencrox avatar Dec 10 '20 10:12 bencrox

I expect everyone will have this issue, 201 is a successful response and endpoints may return 201 or 200 and it could change at any time by whoever runs the endpoint, better to default to 201 allowed because everyone will need to add it in

hst-m avatar Dec 10 '20 12:12 hst-m

I just not sure letting other results to be caught is a good idea. It works, and there exist work arounds. But is it elegant?

202 : some IOT , rest DELETE may return this. 301, 304, 307, 308 : also common to see, even among API.

These are not errors. I am used to think 4XX / 5XX as error.

I can understand 3XX usually lead to extra operations. But I think 202, or in general 2XX, shall be accepted by default

bencrox avatar Dec 12 '20 13:12 bencrox

It should honor the definition of HTTP status code. Which is, 2XX are success, 3XX are redirect, 4XX are client error, 5XX are server error. It should throw error for 4XX/5XX only if it needs to.

ShawTim avatar Dec 12 '20 14:12 ShawTim

This is how the lib was designed. You should pass to bent all the statuses the server might respond:

const client = bent(200,201,202,203,204,301,302...);

This not an issue, but rather a design decision.

aledpardo avatar Apr 13 '21 00:04 aledpardo

Adding a better understandable error is also better for the dev experience. Yesterday I was stuck on.

StatusError: Created
    at ClientRequest.<anonymous> (/home/alex/Documents/DHLParcelModule/node_modules/bent/src/nodejs.js:133:23)
    at Object.onceWrapper (events.js:422:26)
    at ClientRequest.emit (events.js:315:20)
    at ClientRequest.EventEmitter.emit (domain.js:482:12)
    at HTTPParser.parserOnIncomingClient [as onIncoming] (_http_client.js:596:27)
    at HTTPParser.parserOnHeadersComplete (_http_common.js:119:17)
    at TLSSocket.socketOnData (_http_client.js:469:22)
    at TLSSocket.emit (events.js:315:20)
    at TLSSocket.EventEmitter.emit (domain.js:482:12)
    at addChunk (_stream_readable.js:295:12) 
{
  statusCode: 201,
  json: [AsyncFunction],
  text: [Function],
  arrayBuffer: [Function],
  headers: {
    date: 'Tue, 20 Jul 2021 13:55:42 GMT',
    'content-type': 'application/json',
    'content-length': '251',
    connection: 'close',
    vary: 'Origin',
    'x-content-type-options': 'nosniff',
    'cf-cache-status': 'DYNAMIC',
    'expect-ct': 'max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct"',
    'strict-transport-security': 'max-age=15552000; includeSubDomains; preload',
    server: 'cloudflare',
    'cf-ray': '671cb04eea992056-AMS'
  }
}

You have an error! Here is the successful request!

Really confusing in my opinion (at least for me). I'm not against throwing an error. But an error like below should help devs understand why they're getting an error. Maybe just only output this error if there is a 2XX HTTP code.

The request returned status code 201. Status code 201 is not allowed for this request.

AlexSpelt avatar Jul 21 '21 07:07 AlexSpelt

@AlexSpelt that's a good point.

I liked the first part of your proposed error message. What do you think of rephrasing the 2nd part a bit, like:

... 201 status code was not set as an expected response status.

aledpardo avatar Jul 21 '21 12:07 aledpardo

That is even better indeed. Except for the "201 status code" part instead of "status code 201".

But that might be the language barrier for me. English is not my native language.

AlexSpelt avatar Jul 21 '21 12:07 AlexSpelt