!!!BUGFIX: Make any exception handable in renderingGroups by statusCode

[daniellienert]

Before only exceptions that derive from FlowException could be handled with renderingGroups. This sets the status code for unknown exceptions to 500, so they will match a matchingStatusCodes configuration. Therefore a configuration like this will now also render generic exceptions as if they were FlowExceptions with a status code of 500:

Neos:
  Flow:
    error:
      exceptionHandler:
        renderingGroups:

          'allExceptions':
            matchingStatusCodes: [500]
            options:
              templatePathAndFilename: 'some-path'

Note: This is slightly breaking if you handled Flow Exceptions differently than generic exceptions. If you do want to render Flow exceptions differently then generic exceptions, the way to do this is:

Neos:
  Flow:
    error:
      exceptionHandler:
        renderingGroups:

          'flowExceptions':
            matchingExceptionClassNames: ['FlowException']
            options:
              templatePathAndFilename: 'some-path'

          'notFound':
            matchingStatusCodes: [404]
            options:
              templatePathAndFilename: 'specific-code-path'

          'otherExceptions':
            matchingExceptionClassNames: ['Exception']
            options:
              templatePathAndFilename: 'some-other-path'

The first matching group will be used.

[bwaidelich]

but is this really considered a bug?

Good point, definitely a feature

[daniellienert]

@bwaidelich, @albe: I still would argue for "bug". The feature is already there, you can define a rendering group for 500s. But they are only captured if the exception thrown derives from a Flow exception. Which is not expected.

[albe]

I would suggest to fix AbstractExceptionHandler::resolveRenderingGroup() instead so that it respects rendering groups without matchingStatusCode and matchingExceptionClassNames and I'm happy to help with that if you guys agree.

I agree. But just to get it right. With this change as is, you would configure e.g.

Neos:
  Flow:
    error:
      exceptionHandler:
        renderingGroups:

          'flowExceptions':
            matchingStatusCodes: [500]
            options:
              templatePathAndFilename: 'some-path'

And that would then also render ALL exceptions that are not FlowExceptions with this one template as if they were 500 FlowExceptions. With your suggestion, you would need to configure two renderingGroups to cover both cases. Now the question is: which is more reasonable? Ease of use vs. more explicit and flexible config

[albe]

Guys, can we come to any conclusion on this?

I looked at it again, and the change still does make sense IMO - a generic Exception can be assumed to equal a 500 status code. When configuring a rendering group via the status code matcher, it is only sensible to also capture non-FlowExceptions. If you do want to render Flow exceptions differently then generic exceptions, the way to go is IMO:

Neos:
  Flow:
    error:
      exceptionHandler:
        renderingGroups:

          'flowExceptions':
            matchingExceptionClassNames: ['FlowException']
            options:
              templatePathAndFilename: 'some-path'

          'notFound':
            matchingStatusCodes: [404]
            options:
              templatePathAndFilename: 'specific-code-path'

          'otherExceptions':
            matchingExceptionClassNames: ['Exception']
            options:
              templatePathAndFilename: 'some-other-path'

The first matching group will be used. Again this is intuitive.

Regarding the "actual" bug, that nothing is used if no matcher is specified... well, yes. That should be fixed too.

[albe]

ping @bwaidelich

[bwaidelich]

I think the new behavior is pragmatic and makes sense, but it is a potentially breaking change if someone currently handles Flow exceptions differently from other exceptions. This should be documented IMO

[mhsdesign]

Now the question is: which is more reasonable? Ease of use vs. more explicit and flexible config

... just pointing out we have yaml with fancy stuff ...

Neos:
  Flow:
    error:
      exceptionHandler:
        renderingGroups:

          'statusCode500FlowExceptions':
            matchingStatusCodes: [500]
            options: &statusCode500Rendering
              templatePathAndFilename: 'some-path'

          'statusCode500OtherExceptions':
            matchingExceptionClassNames: ['Exception']
            options: *statusCode500Rendering

i think in general these exception groups are hard to configure (due to all the merging - and no possible way (jet) to sort via like position: end) - what @bwaidelich said about the "bug", that occurs when no matcher is defined, i think that is fine - since youd want a matcher - you can always write matchingExceptionClassNames: ['Throwable'] to catch all ...

in the end i come to the conclusion that i have nothing to conclude