koa icon indicating copy to clipboard operation
koa copied to clipboard

default response status code should be set at the end

Open dolsup opened this issue 6 years ago • 6 comments
trafficstars

My suggestion: Default response status code should be set at the end of response, not before route handlers run.

In my case, I'm trying to catch custom error object(Boom) thrown from handlers and set response status by error type at middlewares. But I always get default 404 unless res.body is set, and I don't know whether the 404 is set by default or my handlers or middlewares.

a known workaround does not work now.

dolsup avatar Apr 24 '19 10:04 dolsup

You can make a PR if you want, but I'm afraid this might heavily break applications relying on current behaviour.

fl0w avatar Apr 25 '19 07:04 fl0w

It can be implemented as a form of app-wide option for compatibility. but I'm not sure if it can be approved because there is no app-wide option in Koa for now.

And I found out a workaround can solve my case by just adding a middleware at the first which is set status to undefined.. so this issue is not urgent but IMHO it should be default behavior in the next major version.

dolsup avatar Apr 26 '19 06:04 dolsup

This doesn't work anymore because of this code in response.js

    assert(Number.isInteger(code), 'status code must be a number');
    assert(code >= 100 && code <= 999, `invalid status code: ${code}`);

turbobuilt avatar Jul 19 '19 15:07 turbobuilt

@turbobuilt Oh sorry. I found that I did it with a different method from I described above. In short, I just make every 404 error using Boom so I could distinguish 404 which I throw from default 404. I deal with it in middleware.

I think we should be able to distinguish whether status is default at least for now. This is quite important issue. I'll try to make PR soon.

dolsup avatar Jul 22 '19 01:07 dolsup

Ah, good idea. I think that it is very important as well for UX, because it really frustrated me getting 404s when I was starting simply because I didn't return any data.

turbobuilt avatar Aug 13 '19 16:08 turbobuilt

I stumbled upon this issue yesterday. While I hope it gets fixed, that was my workaround: (I changed the example a little for this issue)

some-file:

// ...
context.throw([code], [message], { statusCode: [code] });
// ...

on error:

app.on('error', (error, context) => {
        // ...
	if (error.statusCode >= 500) {
	// ...
        }
        // ...
});

LeonnardoVerol avatar Jul 07 '20 09:07 LeonnardoVerol