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

refactor: use subset of http-status #27

Merged
merged 2 commits into from Jul 26, 2021
Merged

refactor: use subset of http-status #27

merged 2 commits into from Jul 26, 2021

Conversation

snyamathi
Copy link
Contributor

This PR addresses #26 by taking a subset of the status codes from http-status which contains a bunch of other unused entries which are contributing to bloat: https://unpkg.com/http-status@1.5.0/lib/index.js

This drops the total size of the package from 22.5 KiB to just 3.2 KiB without changing the message returned for each status code.

This would technically change the functionality if someone passed one of the non-numerical keys that are part of the http-status library, though it seems odd to have an error with e.statusCode = '401_CLASS' - it would become a 500 error after this change.

The documentation specifically calls out the status code as being a number so I'm not concerned about it. The docs even specify that the statusCode "Must be greater or equal 400" but I'm keeping the other error codes as well since it's hardly any weight and that very well could be a breaking change for some people.


I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.

package.json Outdated Show resolved Hide resolved
@lingyan
Copy link
Member

lingyan commented Jul 26, 2021

This would technically change the functionality if someone passed one of the non-numerical keys that are part of the http-status library, though it seems odd to have an error with e.statusCode = '401_CLASS' - it would become a 500 error after this change.

Agree 401_CLASS is odd. For this kind of syntax, should we parse the status code from such string, instead of changing to 500? Things like METHOD_NOT_ALLOWED will still have different behavior. Hopefully no one uses those.

@snyamathi
Copy link
Contributor Author

I think that since the docs are pretty explicit that the status code should be a numerical code I'm okay with the potential to break some people https://github.com/yahoo/fumble#api-docs who are doing otherwise.

It's actually better this way since you'd get e.statusCode = 500 instead of e.statusCode = 'METHOD_NOT_ALLOWED' which makes no sense. It was possible for fumble to return a non-numerical status code before which we will fix with this change.

@lingyan
Copy link
Member

lingyan commented Jul 26, 2021

sounds good to me.

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

Successfully merging this pull request may close these issues.

None yet

5 participants