Response returning from presenter

Toanir

Hi!

I was thinking about the means of sending response from the presenter. There are methods as sendResponse, sendJson, etc but I feel like there could be an easier way. I thought: What if it was possible to avoid AbortException and return the response directly? That's how I came up with following proposed structure: https://github.com/…ccc47d47855f

This change effectively enables Presenter::action* methods to return Response directly or return scalar/array which will be turned into JsonResponse. With current implementation of Nette\Application\UI\Presenter::run, the response is still handled via sendResponse method, which could be changed later on.

This also allows component handle* methods to return results in similar way – often we need components to just return JSON response on ajax requests or send FileResponse in case the component, for example, is to print an invoice.

In majority of cases this is done by

<?php
    ...
    $this->getPresenter()->sendJson([
        'status' => 'ok',
    ]);
    // or
    $this->getPresenter()->sendResponse(new FileResponse('dickbutt.jpg'));
    ...
?>

which could be possible to just

<?php
    ...
    return [
        'status' => 'ok',
    ];
    // or
    return new FileResponse('dickbutt.jpg');
    ...
?>

This is achievable thanks to changing internal API by:

returning result of Nette\Application\UI\Component::tryCall – possible BC break – return instance instead of bool,
storing tryCall result of signal handler through Nette\Application\UI\Component::signalRecieved
(i'm not entirelly happy about storing the result in protected property of Component, however this is the best I could come up so far to keep the api of Nette\Application\UI\ISignalReciever)

What do you think about this API which allows to drop dependency of childs component on presenter to send response?

Last edited by Toanir (2019-08-23 00:14)

Marek Bartoš

Looks interesting. You should create a PR so it doesn't get lost in forum history

Toanir

Done (cross-link: https://github.com/…ion/pull/228). I went through contributing guide which suggested to discuss the changes here.

dkorpar

I like this way better, but doubt it will get in.
It's way how most of other frameworks are solving this.
I find it cleaner like this, but it's kinda wierd in nette since you have action*, render*
if U return view response on action what does this mean for render* I think this is currently in colission with presenter lifecycle and I believe because of this it won't be implemented. Plus this is big change which won't go out in minor version so this even if it goes through won't be out for few years probably…

Toanir

Interesting stance. I don't consider this change to be of much impact as far as architectural design is concerned.

Imho this only increases Presenters “thoughtfulness” about result of try-called methods which allows the methods themselves to be simpler and instead of “reaching out” to dependencies and other methods, they can just return the result to be handled.

The return new Response(...) is direct equivalent of $this->sendResponse(new Response()) except it uses language-typical construction to clearly state end of the method.
This is especially beneficial to IDEs who aren't aware that sendResponse is the end and sometimes erroneously evaluate variable to be uninitialized.

The response-sending action could technically be wired even to the render* methods, which is another subject of discussion. I feel that writing return new Response(...) clearly states same intent as $this->sendResponse – I want application to return/send this instance of response for this request that is currently being handled.
The added value is easier syntax and avoiding necessity to use exceptions.

Thanks for your comments, but I feel different about the scale of this change – it only extends support for handling responses and technically is a syntactic sugar that actually helps to avoid headaches with following cases:

try {
  ...
  $this->sendResponse(new FileResponse('bananaphone.midi'));
} catch (\Throwable $ex) {
  // AbortException is thrown within the method and might be accidentally caught
  //  here - needs to be rethrown
  echo 'something is wrong, sorry';
  exit;
}

try {
  ...
  return new FileResponse('bananaphone.midi');
} catch (\Throwable $ex) {
  // AbortException is thrown after the method is exitted in the Presenters run
  //  method itself so we don't have to worry about accidentally catching AbortException
  //  if we just return the response
  echo 'something is wrong, sorry';
  exit;
}

EDIT: I just processed my negative case in my head and it's actually invalid because returned result is within happy-day block of try-catch.

Last edited by Toanir (2019-08-24 09:57)