Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API for adding custom status codes #5

Closed
jonathanong opened this issue Oct 8, 2014 · 9 comments
Closed

API for adding custom status codes #5

jonathanong opened this issue Oct 8, 2014 · 9 comments

Comments

@jonathanong
Copy link
Member

because right now it requires adding like 3 properties manually

@Fishrock123
Copy link
Member

👍

@hacksparrow
Copy link
Member

With reference to jshttp/http-errors#19, if we want to support custom error codes (and maybe other response codes as well), do we accept all integer values as valid? Right now anything not in the array of IANA code are rejected as invalid.

@dicearr
Copy link

dicearr commented Sep 25, 2017

@dougwilson @jonathanong Can I tackle this?

@dougwilson
Copy link
Contributor

I don't think this is relevant any longer. It was originally opened because the http-errors module would not allos codes that were not listed here. That is not correct behavior. The http-errors module has since been updated to allow custom codes, where this module is used as a reference for known codes only.

What use-case remains to require this change? If custom codes are added which alters the global state, that may cause poor interactions with other users of this module within the same process.

Part of this would be understanding what the purpose of this module really is: it is simply a collection of the known standard codes for reference or is it an enforcer that will restrict use of any code not listed here by a process? I tend to think of it as the former rather than the latter.

@dicearr
Copy link

dicearr commented Sep 25, 2017

I have reached this issue through koajs/koa#1042. The issue is still opened but I don't know if this is necessary.

@dougwilson
Copy link
Contributor

I see. One issue with at least the last code example there is that it assumes the statuses module resolved by the user's require and the one within Koa will be the exact same instance, in order to achieve the action-at-a-distance effect (registering with statuses magically will affect Koa operation). Koa is certainly free to implement a strict API that requires the code to be on this list, but I'm not sure I agree that to make that configurable should place the burden on the list curator.

The HTTP spec states that any code is valid as long as it is in range. By that very nature that means this module doesn't validate codes by them being listed here, only asserts that there is data about the code here, not if it is valid.

@dicearr
Copy link

dicearr commented Sep 25, 2017

I see your point of view. Even so, you can add new status codes by rebuilding the package. How harmful would be to be able to add new ones dynamically (with or without validation)?

@ryhinchey
Copy link

I have reached this issue through koajs/koa#1042. The issue is still opened but I don't know if this is necessary.

@dicearr that issue is now closed. Here's the relvant PR on Koa's side https://github.com/koajs/koa/pull/1308/files

Part of this would be understanding what the purpose of this module really is: it is simply a collection of the known standard codes for reference or is it an enforcer that will restrict use of any code not listed here by a process? I tend to think of it as the former rather than the latter.

Of anyone in this thread, I probably have the least amount of experience with this module but I also think of this module's purpose as the former.

With reference to jshttp/http-errors#19, if we want to support custom error codes (and maybe other response codes as well), do we accept all integer values as valid? Right now anything not in the array of IANA code are rejected as invalid.

It looks like this was resolved here jshttp/http-errors@4b45191

Perhaps this issue can be closed now?

@ryhinchey
Copy link

I'm going to go ahead and close this issue. Feel free to re-open if there's an actual issue here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants