V4: routes can no longer return null?

[mnapoli]

I had this route:

        $app->get('/', function () {
            // nothing
        });

It doesn't seem to work anymore with v4.0.0:

TypeError : Return value of Slim\Handlers\Strategies\RequestResponse::__invoke() must implement interface Psr\Http\Message\ResponseInterface, null returned

Is this something that changed in v4?

I read both the documentation and the upgrade guide and couldn't find any mention of this.

[shadowhand]

The Response docs are the only mention I could find:

Your Slim app’s routes and middleware are given a PSR-7 response object that represents the current HTTP response to be returned to the client.

[adriansuter]

Yes, every route handler has to return a response.

[shadowhand]

This should probably documented as a side effect of adopting PSR-15, in which all request handlers must return a PSR-7 ResponseInterface.

[mnapoli]

OK thanks. This should indeed be mentioned in the upgrade guide as this is a major BC break.

[l0gicgate]

That is because we type hinted the return value of the strategy. I think in Slim 3 it was not.

Either way, why is this necessary? Just return a blank response object if you really need to.

[mnapoli]

Or yes maybe we could re-instate the previous behavior that allowed this? That would be one less BC break for users, which is good.

[shadowhand]

Was the fact that handler could return nothing documented in v3? Unless that was documented behavior, I feel like that was probably not meant to be a feature.

[shadowhand]

I guess the handling of echo implicitly allows for returning nothing, per the v3 callable docs:

There are two ways you can write content to the HTTP response. First, you can simply echo() content from the route callback. This content will be appended to the current HTTP response object. Second, you can return a Psr\Http\Message\ResponseInterface object.

[shadowhand]

The solution for RequestResponse and RequestResponseArgs is pretty simple, just add:

return /* ... */ ?? $response;

I do not think that should be added to RequestHandler because RequestHandlerInterface requires a ResponseInterface return value.

[l0gicgate]

@shadowhand we’d need to check and see if the return from $callable($request, ...) is a ResponseInterface from within RequestResponse also I’m not sure if we may need to modify the ResponseEmitter in order to accommodate users who want to use “echo”.

[shadowhand]

@l0gicgate the /* ... */ in my example is the existing $callable invocation. If it returns null or void, then the existing $response is returned; for example RequestResponse would be:

return $callable($request, $response, $routeArguments) ?? $response;

[l0gicgate]

@shadowhand how do we deal with the logic in ResponseEmitter? Imagine we return an empty $response but the user used echo within the route callable, then ResponseEmitter will assume that the $response object is empty when it theoretically isn't. This is one reason why we type hinted the strategy.

[shadowhand]

I don't see why that would matter. The difference between a callable handler that returns nothing and one that returns an empty response is:

 function ($request, $response) {
-    return $response;
 }

The empty response detection is exactly the same in either situation.

[l0gicgate]

That's exactly my point, if someone uses echo "content outside response object"; from within a route callable, isResponseEmpty() only checks the Response object and not the actual output buffer which is where the content from the echo statement would appear.

There's logic associated with a response actually being empty (which it wouldn't be if someone used echo from within a route callable): https://github.com/slimphp/Slim/blob/84c96a3d1dcddd84633b89720fdde827a058a761/Slim/ResponseEmitter.php#L39-L43

[mnapoli]

@l0gicgate

how do we deal with the logic in ResponseEmitter?

I would say the same way it was dealt with in Slim 3?

To me the question boils down to: should this BC break be kept or reverted?

[shadowhand]

@l0gicgate isn't echo'd content handled by the output buffer middleware?

https://github.com/slimphp/Slim/blob/2b0ed80b2ab4acfb5e7648797b8202e4d9aea06d/Slim/Middleware/OutputBufferingMiddleware.php#L58-L61

In which case, the response will already have been filled with echo'd content by the time it reaches the ResponseEmitter ?

[l0gicgate]

@shadowhand OutputBufferMiddleware is an optional middleware it’s not used internally.

@mnapoli I don’t personally like enabling a null return without justification. If it’s just for convenience, I don’t see the purpose.

[mnapoli]

@mnapoli I don’t personally like enabling a null return without justification. If it’s just for convenience, I don’t see the purpose.

This isn't for convenience, this is a major BC break. At the moment this BC break isn't documented or acknowledged.

Limiting the amount of additional BC breaks to this release (that is already quite huge) could be a good thing.

[l0gicgate]

@mnapoli I'm fine with acknowledging it. I'm also fine with reverting it if you can provide a solid argument as to why we should enable users to return a null response.

[mnapoli]

Understood.

if you can provide a solid argument as to why we should enable users to return a null response.

It is a BC break. V4 is a big BC break. Being able to remove one BC break from V4 is a good thing. That's all I've got :)

To be clear I don't like returning null as well :)

[shadowhand]

@l0gicgate I realize it is optional, but what difference does that make? Using echo and not using the output buffer middleware is supposed to do... what exactly? How does that impact this issue?

[l0gicgate]

To be clear I don't like returning null as well :)

I guess we're agreeing then. I'm closing this as won't fix, I will address the BC break in the 4.1 Release so it's on record.

[fabswt]

I don't think it's documented anywhere. The fact it worked in v2 and v3 is reason enough to fix it and/or document it.

[fabswt]

An argument for allowing not returning a response from the route might be all the examples out there (from official docs), and the developer's need to quickly brush up a test route.

Consider these examples from official Slim docs. They do not return a response:

From Slim-Flash:

$app->get('/bar', function ($req, $res, $args) {
    // Get flash messages from previous request
    $messages = $this->flash->getMessages();
    print_r($messages);

    // Get the first message from a specific key
    $test = $this->flash->getFirstMessage('Test');
    print_r($test);
});

From Slim 4 itself:

$app->get('/hello/{name}', function (Request $request, Response $response, $args) {
    $name = $args['name'];
    echo "Hello, $name";
});

Ultimately, the main argument I see is make the developer's life easier, which is the whole point behind a framework (at least it was for me when I chose Slim 2.) This error is a distraction.

Can let the framework fall back to a default value when no response is provided. Ensures backward compatibility with Slim 2 and 3.

[juliangut]

@fabswt in Slim 4 you can always register a custom Slim\Interfaces\RequestHandlerInvocationStrategyInterface in which you can handle the return value from your routes, such as what is done in juliangut/slim-routing

I've just added the null return control thanks to this issue (https://github.com/juliangut/slim-routing/blob/master/src/Strategy/ResponseTypeStrategyTrait.php#L76)

[XedinUnknown]

Still says you can just echo, but in fact I get:

Return value must be of type Psr\Http\Message\ResponseInterface, null returned