MarcinOrlowski / laravel-api-response-builder

Builds nice, normalized and easy to consume REST JSON responses for Laravel powered APIs.
MIT License
721 stars 79 forks source link

Setting status code for the response. #202

Closed magdi14 closed 3 years ago

magdi14 commented 3 years ago

I've been using the package for 3 months now and i noticed that if the request is returning any status -except- 200 it fails to set the response status with the returning status, let's say if we're returning 404 the response builder will format the response json with code, etc ... but the response status it self is not as the returning code, it gives 200, so if this a misunderstanding from me of is this feature not supported?

MarcinOrlowski commented 3 years ago

Can you show how you call RB in this particular case?

magdi14 commented 3 years ago

I just want to ask if "withHttpCode()" method is setting the response status code or just adding the code formatting in the returning json?

On Thu, Jun 17, 2021 at 2:28 PM Mohamed Magdy @.***> wrote:

Hi there, Kindly find the attached image of handling the response.

Thanks

On Thu, Jun 17, 2021 at 2:05 PM Marcin Orlowski @.***> wrote:

Can you show how you call RB in this particular case?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MarcinOrlowski/laravel-api-response-builder/issues/202#issuecomment-863181956, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGEWILVRIT5ID5ADTRI47IDTTHQH3ANCNFSM463O7GGA .

magdi14 commented 3 years ago
Screenshot 2021-06-17 142706
MarcinOrlowski commented 3 years ago

withHttpCode() is HTTP status code and is passed to returned HTTP response without much filtering on the library side. What is added to JSON is refered apiCode().

However it looks you use the same value $content['code'] for both payload (code in JSON response) and as HTTP code. This is most likely your culprit as HTTP codes cannot be freely chosen. See: https://www.askapache.com/net/http-status-codes/ for their meaning. For successful responses you should most likely stay between 2xx-3xx range, for errors in 4xx. In fact if you do not need to have specified code, remove withHttpCode() completely and let ResponseBuilder do that for you.

magdi14 commented 3 years ago

image

i did what you said and removed withHttpCode method but it still give me this response 200 ok but the returning code is 404

MarcinOrlowski commented 3 years ago

First, please do not post images of text as this is hard to deal with. Just post code as plain text (paste it here, select with mouse and click <> to format as code).

I am for now quite confident I have such cases covered by tests. While this still does not mean there cannot be bug I am not really convinced at the moment. Mostly because your code contains 3 branches, one asSuccess() and 2 asError(). I suspect that real flow is different than you think. But to be sure do som extra testing and either isolate the call so you have just one, or just replace withData() in the above code with something unique (i.e. withData('success') for first call, withData('error1') for second andwithData('error2')` for third. Or set a breakpoints. Anything you want to catch offensive call. And then check if the response it produced matches. If not, you should be able to reproduce the issue with exactly the same data -> just post it then so I can repro too.

MarcinOrlowski commented 3 years ago

Finally, note, that unless you changed the code, valid ResponseBuilder produced structure is like this:

{
   "success": false,
   "code": 250,
   "locale": "en",
   "message": "Error #250",
   "data": null
}

you do not have locale nor message. So also check your route is correct (or use ExceptionHandlerHelper).