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

Unable to return custom response in the API #2076

Closed
hdodov opened this issue Sep 12, 2019 · 4 comments

Comments

@hdodov
Copy link

commented Sep 12, 2019

Describe the bug
I want to return an empty response along with a message, but I can't find a way. If I return a new Response object, I get an error.

To Reproduce

  1. Add the following route:
[
    'pattern' => 'data',
    'method' => 'GET',
    'action' => function () {
        return new Response('empty body', null, 204);
    }
]
  1. Open this route in the browser
  2. Error:
{
    "status": "error",
    "route": "export",
    "exception": "Kirby\\Exception\\NotFoundException",
    "message": "The object \"response\" cannot be resolved",
    "key": "error.notFound",
    "file": "C:\\xampp\\htdocs\\kirby\\kirby\\src\\Api\\Api.php",
    "line": 410,
    "details": [],
    "code": 404
}

Expected behavior
There should be a way to easily return all HTTP response codes from a custom route.

Kirby Version
3.2.3

Additional context
I read this forum post and it doesn't work for me. If I echo the Response, I get:

{
    "status": "error",
    "message": "not found",
    "code": 404
}

Because the return value form the route is null. If I return true, I get status OK and response code 200, i.e. the echoed Response is ignored.

I think two things should change:

  1. You should be able to return new Response() in the route and have that work correctly
  2. The Response object should be able to handle a message parameter so you can easily send a message along with the status code like this:
return new Response([
    'code' => 204,
    'message' => 'That object is empty.'
]);

...instead of:

return new Response([
    'code' => 204,
    'body' => json_encode([
        'status' => 'ok',
        'message' => 'That object is empty.',
        'code' => 204
    ])
]);
@lukasbestle

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2019

I agree on the first part (support for returning response objects from API routes), that would probably be rather simple to add. It's a new feature however and not a bug.

I'm not so sure about the second part though. Response objects are pretty raw data objects. You can throw any kind of response structure at them, so I think that it shouldn't care whether there is a message key in a JSON array etc. If you wanted a JSON array, you would return it directly from the route. If you want full flexibility, you can return a completely custom response with the first feature you proposed.

@hdodov

This comment has been minimized.

Copy link
Author

commented Sep 13, 2019

If you wanted a JSON array, you would return it directly from the route.

Yes, but you wouldn't be able to set the status code? I can do this:

throw new Exception('My message', 403);

To throw a 403 error with some message. I want to be able to do the same, but for successful responses. Anyway, I think that's a pretty niche case.


I also forgot to propose setting the status code by returning a number. For example:

return 204;

...sets the status code to 204. That could be pretty handy.

lukasbestle added a commit that referenced this issue Sep 13, 2019
@lukasbestle

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2019

I also forgot to propose setting the status code by returning a number.

That only ever makes sense for 204 though, right? Every other response generally comes with a body, which you wouldn't be able to pass when just returning the response code.

I have implemented the "custom response" feature now (#2080), so you will be able to do this:

return new Response(null, null, 204);
@lukasbestle lukasbestle added this to the 3.2.5 milestone Sep 13, 2019
@lukasbestle lukasbestle self-assigned this Sep 13, 2019
bastianallgeier added a commit that referenced this issue Sep 13, 2019
@bastianallgeier

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2019

afbora added a commit to afbora/kirby that referenced this issue Sep 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.