flow-development-collection icon indicating copy to clipboard operation
flow-development-collection copied to clipboard

Exceptions should respect the requested content type

Open neos-bot opened this issue 10 years ago • 11 comments

Jira issue originally created by user @bwaidelich:

The Flow ExceptionHandler s currently only adjust their rendering based on request type (Web/CLI) and context (DebugExceptionHandler/ProductionExceptionHandler).

They should, in addition, be able to react upon the requested format, e.g. by evaluating the Accept HTTP header of the current request.

Jira-URL: https://jira.neos.io/browse/FLOW-202

neos-bot avatar Feb 16 '15 10:02 neos-bot

Comment created by @bwaidelich:

Note: We "hotfixed" this for Neos 1.1.* with NEOS-108 but we need a general solution in Flow (see comments on NEOS-108 for previous discussions)

neos-bot avatar Feb 16 '15 10:02 neos-bot

Comment created by sorenmalling:

[~bwaidelich] So, I did some looking into this yesterday.

I focused on the ExceptionHandler classes. My idea is to add a configuration similar to matchingStatusCodes and matchingExceptionClassNames for the mediatype

What puzzles me, is that I have no connection to the Request object, so I can't see what media type is accepted.

In that way, a configuration like to following could be an example

TYPO3:
  Flow:
    error:
      exceptionHandler:
        renderingGroups:
          'someFancyGroupName':
            matchingExceptionClassNames: ['TYPO3\Flow\Persistence\Doctrine\Exception\DatabaseException']
            matchingMediaType: 'application/xml'
            options:

neos-bot avatar Apr 15 '15 10:04 neos-bot

Comment created by sorenmalling:

So, the actual question is:

How do I get the accepted mediatype for the request, when Request is not available?

Can I somehow inject it or just initialize it?

neos-bot avatar Apr 15 '15 10:04 neos-bot

Comment created by @bwaidelich:

[~sorenmalling] I had a similar idea and I like it, although it should be matchingMediaTypes (plural) IMO. Getting hold of the Request is indeed a problem we need to solve.. We can get it via

/****
 * @Flow\Inject
 * @var \TYPO3\Flow\Core\Bootstrap
 ****/
protected $bootstrap;

// ...

$httpRequest = $this->bootstrap->getActiveRequestHandler()->getHttpRequest();

..That's not the best solution in terms of clean coding, but it should work for our case

neos-bot avatar Apr 15 '15 10:04 neos-bot

Comment created by @bwaidelich:

..but, true, a problem remains, because we don't know what the negotiated format is, because that's set by AbstractController::initializeController() in the ActionRequest..

neos-bot avatar Apr 15 '15 10:04 neos-bot

Comment created by pumatertion:

I am still pretty unsure about the response format. Except cli response the format should be detected by request header data instead of extension .whatever? Example: Accept:text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,/;q=0.8

neos-bot avatar Apr 15 '15 11:04 neos-bot

Possible ways to solve/look at this:

  • the media type can only be used, once it is "negotiated" in the normal way. This happens in initializeController currently, but could be done a little bit earlier. Still, this means that exceptions return type depends on how deep into the bootstrap they happen.

  • the ComponentContext is created early inside the RequestHandler and holds a reference to the Request, so this could be made available for injection into the ExceptionHandler (which will be initialized later in the handleRequest() invocation).

  • just create a new Request::fromEnvironment() in the exceptionHandler and deal with media type from there.

  • exception handling is so low-level, that it should just read the media type directly from $_SERVER['HTTP_ACCEPT'] value and not depend on an HTTP abstraction via Request

The bottom three options all mean, that the negotation of the format needs to happen somewhere in the error handling (again).

albe avatar Apr 30 '18 09:04 albe

@albe Thanks for the input! I think we should make the ComponentContext (or some abstraction) available "globally" (for http requests). This would solve other issues too (for example this could be a replacement to fetch the HTTP request. Instead of using Bootstrap or creating a new instance(!)).

The content type negotiation could be moved to a HTTP component. IMO this makes more sense than doing this in the controller anyways.

bwaidelich avatar Apr 30 '18 09:04 bwaidelich

Sounds like two good actionable plans. Let's create new issues for those and link them here as dependencies.

albe avatar Apr 30 '18 12:04 albe

Like mentioned in https://github.com/neos/flow-development-collection/issues/1305#issuecomment-696009521 I no longer think that my suggested approach is very feasible (and with the Middleware introduction the ComponentContext will no longer be accessible globally).

Instead I would suggest to go the "easy route" and just implement some minimal content type negotiation into the exception handling (possibly extensible via settings)

bwaidelich avatar Sep 21 '20 09:09 bwaidelich

maybe it would be a good fit to fix this with 9.0 since its a bit breaking since this behaviour has been now like ever :P

Especially the neos ui would profit from this https://github.com/neos/neos-ui/issues/3183 as its currently uses the dom parser and innerText to get simple details :O

mhsdesign avatar Jan 29 '24 21:01 mhsdesign