search

jshttp / content-type

Create and parse HTTP Content-Type header
MIT License
131 stars 25 forks source link

Add documentation on error handling, specify missing header in error message #1

Closed gr0uch closed 9 years ago

gr0uch commented 9 years ago

Note: in my opinion it should not throw as many errors as it does, allow it to return null values and then it is up to the programmer to check if it succeeded. This is particularly annoying in the case when a request does not have Content-Type specified.

dougwilson commented 9 years ago

This is particularly annoying in the case when a request does not have Content-Type specified.

Gotcha. We should probably add a content-type getter function which returns the content-type or undefined if it's not there.

rlidwka commented 9 years ago

Please remove dotfiles from this PR because:

  1. It should be a separate pull request
  2. They should be added through this repository first
  3. .npmignore is completely useless because this project uses files property
  4. Adding .jshintrc was discussed before and decided against
dougwilson commented 9 years ago

P.S. @Fishrock123 , can you update the labels for this repo :)?

gr0uch commented 9 years ago

@dougwilson if a key on an object is missing, it already returns undefined and the rest of the function will still throw an error anyways. I just don't want to have to wrap this in a try...catch block because of a missing header :(

dougwilson commented 9 years ago

@daliwali I know. I'm talking about making a new function exported by this module that doesn't do that :)

dougwilson commented 9 years ago

@daliwali how does this API look:

var contentType = require('content-type')
var type = contentType(req)

// type is a parsed type or undefined
gr0uch commented 9 years ago

@dougwilson I like that, very concise.

dougwilson commented 9 years ago

P.S. @Fishrock123 , can you update the labels for this repo :)?

dougwilson commented 9 years ago

@daliwali sorry to bug you again :) But for the contentType(req) API, in your opinion, what should it do if there is a Content-Type header, but it's invalid?

gr0uch commented 9 years ago

@dougwilson Ah, perhaps it really should throw an error. The alternative is to fail silently, which wouldn't feel good either. Throwing an error would feel consistent with some other parsing methods, like JSON.parse. Ok I take it back, errors are fine all around. It was just disorienting to see argument string is required to be a string when I passed in a request object.

dougwilson commented 9 years ago

Ok, we can think on it some more. For now, I'm' going to release your changes from this PR; feel free to open a new issue for anything :)