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

fail should return HTTP Status! #11

Open
Baterka opened this issue Feb 23, 2019 · 8 comments
Open

fail should return HTTP Status! #11

Baterka opened this issue Feb 23, 2019 · 8 comments

Comments

@Baterka
Copy link

Baterka commented Feb 23, 2019

I think fail should return HTTP error status too...

For example: res.status(500).jsend.error({code: 500, message:"Internal Server Error"})

@Prestaul
Copy link
Owner

@Baterka are you really referring to fail which does not use a code, or are you saying you think that, like your example, if you provide a code with an error (which is not required for jsend errors) that the status-code should be set on the response automatically?

Sorry, I'm not 100% certain that I understood the request.

@Prestaul
Copy link
Owner

Closing for lack of clarification.

@hoyikxian
Copy link

@Prestaul I think what he's trying to say is that if he hits an internal server error, jsend should return the HTTP status as 500 error instead of 200 OK. Because right now as I am using JSend, that's what i realised, is that any response is a 200 OK

@Baterka
Copy link
Author

Baterka commented Aug 2, 2019

@hoyikxian Exactly what I am trying to say

@Prestaul
Copy link
Owner

Prestaul commented Aug 5, 2019

Okay, that makes more sense than what I thought you were saying initially.

This would be a breaking change and I'm thinking it would make the most sense to have have error use 500 and fail use 400 as long as the status code is not already set. Does that makes sense to you, @Baterka and @hoyikxian?

@Prestaul Prestaul reopened this Aug 5, 2019
@artyuum
Copy link

artyuum commented Aug 5, 2019

@Prestaul I'm looking forward to see this feature implemented.

This would be a breaking change and I'm thinking it would make the most sense to have have error use 500 and fail use 400 as long as the status code is not already set.

I'm okay with that.

@Baterka
Copy link
Author

Baterka commented Aug 5, 2019

Okay, that makes more sense than what I thought you were saying initially.

This would be a breaking change and I'm thinking it would make the most sense to have have error use 500 and fail use 400 as long as the status code is not already set. Does that makes sense to you, @Baterka and @hoyikxian?

It makes perfect sense. I understand approach "define statuses only by JSON" (How described in original specification), but it should have right HTTP statuses too as redundant. I forked your repo and did some changes. Merge it if you like it (but edit tests, because they are failing now)
#13

@hoyikxian
Copy link

Okay, that makes more sense than what I thought you were saying initially.
This would be a breaking change and I'm thinking it would make the most sense to have have error use 500 and fail use 400 as long as the status code is not already set. Does that makes sense to you, @Baterka and @hoyikxian?

Yeah that would kind of make sense, but in the future you might want to be more specific with the http error codes. However, I am looking forward to the changes.

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

No branches or pull requests

4 participants