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

err.headers standardization #27

Closed
jonathanong opened this issue Mar 3, 2016 · 4 comments
Closed

err.headers standardization #27

jonathanong opened this issue Mar 3, 2016 · 4 comments

Comments

@jonathanong
Copy link
Member

koajs/koa#668

not sure how we can standardize this... last thing i want is for other people to use a different property name. also want all the text casing to be the same so people don't do:

err.headers = {
  'content-type': 'text',
  'Content-Type': 'application/json',
}

maybe set .headers = {} by default?

@dougwilson
Copy link
Contributor

Hey @jonathanong, yes, I have been reading the discussions on that thread and didn't have any objects to the conversation :) I think it makes sense to get it added here as a standard way, just like err.status is.

err.headers seems like a fine name, and would fit well with the status code property. As for the casing of the keys, that is always a tricky one in Node.js, haha. Especially for response headers, it I pretty useful to have a case-preserving way to provide the headers, even though all-lower-case is easier to access a particular header.

Perhaps what we can do is let a user specify the headers in any case, but filter what actually get attached to the error objects as the headers property to remove any case-insensitive duplicates, essentially this object would be just like res._header in Node.js core.

As for setting err.headers = {} by default, it seems like it's probably fine. I'm still weighing it because I'm thinking "well, allowing err.headers === undefined could signal that the header for this error are "undefined" vs err.headers = {} meaning that there are specifically no headers for the error. But thinking more, I believe the expectation here is that ultimately err.headers would be mixed in to the existing response headers, making the difference between undefined and {} not meaningful.

Thoughts?

@jonathanong
Copy link
Member Author

So we should filter the names in this library? Or at the framework level? Or have this library have a "set headers from error" method?

I think you're right about defaulting headers to undefined. I'm not sure what the practical difference is, but I see it as different intentions.

Cool I think we're in agreement!

@felixfbecker
Copy link

felixfbecker commented Aug 10, 2016

It's also possible to it like the fetch API and let err.headers be a case-insentitive Map-like object like https://developer.mozilla.org/en-US/docs/Web/API/Headers, but still accept an object hash in the constructor, that then gets converted.

https://developer.mozilla.org/en-US/docs/Web/API/GlobalFetch/fetch

@dougwilson
Copy link
Contributor

For now I just added a documented mention of the err.headers

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

3 participants