TYPO3 Core

Helmut Hummel wrote in #note-3:

What exactly do you mean with "missing file"?

1. You are passing a FAL object, but the file is not available in the storage?
2. You are passing null instead of a file object?
3. Something else?

Any case where an exception for missing FAL (f:image, f:uri.image, f:link.file, ...) will cause an exception breaking the whole frontend! This means either FAL-Object is NULL or the file isn't available.

There may be some (minor) cases where this could make sense, but under normal circumstances a missing image or file-download, it makes more sense to display a placeholder or message to the end-user instead of throwing exceptions breaking to display the website.

If should be up to the developer to choose what he thinks is right to do here! I think a Signal/Event System would be a very flexible choice. Additional attributes to control the behaviour in case of FAL-Exception would be a good solution for non-profesionals (TYPO3 Integrators who are not familiar with PHP/Signal/Event, ...).

The current behaving of TYPO3 for these Exceptions is like compared to a car, when the car-radio is suddenly malfunctioned and does not play audio, the whole car breaks while driving on highway on the middle of the road!

Gabriel Kaufmann / Typoworx NewMedia wrote in #note-4:

This means either FAL-Object is NULL or the file isn't available.

These are two different cases, that should be handled separately.

If a file object is passed to the VH, but is not available in the storage, there is FAL missing file fallback handling that should kick in, to avoid exceptions being thrown. If you still get exceptions for that case, that needs to be fixed imho.

If you pass null to the image view helper, that (imho) clearly is a fatal error, that should be exposed with how this VH is currently designed.

I see two options:

1. Change the VH to accept a fallback string that is shown, when no image is passed to the argument and show the fallback in that case (does only work for tag VH, not the URI VH)
2. Wrap calling the VH with an if in the template to avoid calling it with invalid props

1. is something TYPO3 must do obviously, 2. is something you can do right now (and is what I am doing currently im my projects)

Helmut Hummel wrote in #note-5:

I see two options:

1. Change the VH to accept a fallback string that is shown, when no image is passed to the argument and show the fallback in that case (does only work for tag VH, not the URI VH)
2. Wrap calling the VH with an if in the template to avoid calling it with invalid props

1. is something TYPO3 must do obviously, 2. is something you can do right now (and is what I am doing currently im my projects)

Thanks for feedback. Of course the Fluid-IF's would be an option to at least avoid passing NULL-Object. Checking Object-Props with Fluid-If will be mich more complicated and in my opinion this will blow up fluid-templates enormously!

I don't think these options are neither a clean solution nor good practice. Templates will blow up enormously, it will may be confusing to junior developers, too. And never the less doing this checks in the Template is bad practice and separating it into a new ViewHelper like <f:file.checkFalOkay /> seems to be cleaner than fluid, but still is many overload for all this stuff.

What about my proposal handling this inside the FAL ViewHelper themselfes? We talk about minor changes! We need try/catch and then emit the Exception to an Event-Slot for this. There could be two Core Listeners to be choosen:
Either the old behaviour (let the exception throw, f.e. useful in Development or Testing Environment). And a Signal-Handler just Logging and being silent.

Everything else would be up to developers implementing their own use-case just by adding their own Event-Listener.

First of all, thank you for bringing up this issue.

I personally don't think that an event would be a fitting solution here. This would be a special error handling for only one (or a few) ViewHelpers instead of a general solution that could improve the error handling in all ViewHelpers.

From a core view, I think that there should be a more isolated exception handling for Fluid templates. This could be implemented similar to contentObjectExceptionHandler where you can catch exceptions per content element, so that only this content element is "broken".

From a project view, I think that the issue needs to be tackled by the developer. There are certainly cases where a fallback image would be fine, but there are also cases where you'd want to know that something is wrong. While we could add a fallback image option to the image ViewHelper, I don't think that this should be set by default. However, this default image could then also be implemented in your project, for example with f:or or your own fallback ViewHelper.

Simon Praetorius wrote in #note-7:

First of all, thank you for bringing up this issue.

I personally don't think that an event would be a fitting solution here. This would be a special error handling for only one (or a few) ViewHelpers instead of a general solution that could improve the error handling in all ViewHelpers.

From a core view, I think that there should be a more isolated exception handling for Fluid templates. This could be implemented similar to contentObjectExceptionHandler where you can catch exceptions per content element, so that only this content element is "broken".

From a project view, I think that the issue needs to be tackled by the developer. There are certainly cases where a fallback image would be fine, but there are also cases where you'd want to know that something is wrong. While we could add a fallback image option to the image ViewHelper, I don't think that this should be set by default. However, this default image could then also be implemented in your project, for example with f:or or your own fallback ViewHelper.

Thanks for the feedback and participating here. The Signal/Event principle is only one possible idea. I favorize a solution that works without (only) Typoscript. As TYPO3 (finally) figured out it's better to follow industrial standards like Symfony, we should have a PHP specific solution. This could also have some kind of configuration that can be used either by YAML (site-config) or TypoScript. But it should be a PHP solution that can be extended on PHP way to customize.

TO have a more general solution the try/catch could take place here:
\TYPO3Fluid\Fluid\Core\ViewHelper\ViewHelperInvoker

Whereas the "Hook" or Signal then could pass the original exception, the source (ViewHelper-Class) and some kind of Response-Object so that a developer can then decice what to do. It should be possible to emit a Propagation-Stop (f.e. using a StopPropagation Exception) and to send some custom response.

It should be possible to customize everything by ViewHelper type. So if there is a better idea to solve this in generic way without having to fiddle around each viewHelper you're welcome.

Gabriel Kaufmann / Typoworx NewMedia wrote in #note-8:

Simon Praetorius wrote in #note-7:

There are certainly cases where a fallback image would be fine, but there are also cases where you'd want to know that something is wrong.

This is a very important statement, which I would like to rephrase in a more general way:

There are template specific cases, where an error in a view helper can be handled gracefully. But there are also cases where you'd want to know that something is wrong.

For example: In one template/partial/layout, the existence of an image is optional, whereas in another template the existence of an image is required in order to produce a meaningful rendered result.

Can we agree on this?

The Signal/Event principle is only one possible idea.

TO have a more general solution the try/catch could take place here:
\TYPO3Fluid\Fluid\Core\ViewHelper\ViewHelperInvoker

It should be possible to customize everything by ViewHelper type. So if there is a better idea to solve this in generic way without having to fiddle around each viewHelper you're welcome.

If we agree on our statement from above, choosing this implementation idea, does not solve the case where the exact same view helper if fine to fail in one case but not in another, because neither the ViewHelperInvoker, nor the caused exception has any information to distinguish these two cases.

This is why I think we would conceptually need something on a template level to handle errors.

Implementation wise, the ViewHelperInvoker is never called any more for view helpers that implement renderStatic, once a template has been cached, which means catching view helper errors reliably needs a different implementation approach.

Simon Praetorius wrote in #note-7:

From a core view, I think that there should be a more isolated exception handling for Fluid templates. This could be implemented similar to contentObjectExceptionHandler where you can catch exceptions per content element, so that only this content element is "broken".

@Simon Praetorius What do you think about so called error boundaries?

https://www.digitalocean.com/community/tutorials/react-error-boundaries
https://remix.run/docs/en/main/route/error-boundary

A rough idea for a template level API could look like this:

<f:errorBoundary>
    <f:try>
        <f:render partial="Foo" arguments="{_all}}"/>
    </f:try>
    <f:catch error="\Throwable">
        Fallback content
    </f:catch>
</f:errorBoundary>

Helmut Hummel wrote in #note-10:

@Simon Praetorius What do you think about so called error boundaries?

This is a bit more fine-grained than I had in mind, but it's certainly worth thinking about. Probably also worth checking out other frontend frameworks.

My general approach would be something like this (similar to contentObjectExceptionHandler):

10 = FLUIDTEMPLATE
10 {
  # Don't let exceptions break out of template, use default handling
  exceptionHandler = 1
  # use custom handling
  exceptionHandler = Vendor\Namespace\ExceptionHandler\MyExceptionHandler
}

plugin.tx_myextension_pi1 {
  view {
    exceptionHandler = 1
  }
}

The primary goal would be to limit exceptions to the template and would not affect other parts of the page.

This would be a TYPO3 feature and not a Fluid feature, in contrast to your suggestion. I think that both could also co-exist.

@Helmut Hummel well if it is possible to use the approach letting FLUIDTEMPLATE Processor handle the errors using an ErrorHandler then I don't understand why it shouldn't be possible to do it with an Signal/Event approach?

But nevertheless it's much better to do it there than in the template. I don't want to blow-up all my templates with try/catch ViewHelpers around images and doing so I could also wrap the whole template in that try/catch to fetch all. Following this idea... it's just stupid to do it, but do it right in first way and let the template-engine try/catch these errors instead.

Gabriel Kaufmann / Typoworx NewMedia wrote in #note-12:

Following this idea... it's just stupid to do it, but do it right in first way and let the template-engine try/catch these errors instead.

We're discussing potential alternatives here, and I think that every input is valuable here. Your input is valuable, Helmut's input is valuable as well and everyone else is welcome to join the discussion.

It really isn't helpful nor is it part of the TYPO3 culture to classify solutions or opinions as "stupid". Please think about your words, especially since most of us work on TYPO3 in our free time.

Of course it is

Simon Praetorius wrote in #note-13:

We're discussing potential alternatives here, and I think that every input is valuable here. Your input is valuable, Helmut's input is valuable as well and everyone else is welcome to join the discussion.

It really isn't helpful nor is it part of the TYPO3 culture to classify solutions or opinions as "stupid". Please think about your words, especially since most of us work on TYPO3 in our free time.

You're right. Sorry for the stupid, didn't find the right word for it. I didn't want to attack anyone in person, it was just the concept I don't like for modern coding principles. Sorry again for the bad word.

Simon Praetorius wrote in #note-11:

This is a bit more fine-grained than I had in mind

Sure it's more fine grained. And I also agree it's verbose. Maybe we could figure out a less verbose version...

My general approach would be something like this (similar to contentObjectExceptionHandler):

Since FLUIDTEMPLATE is a content object, this would straight up duplicate its functionality. contentObjectExceptionHandler can already deal with individual content object definitions (see: https://docs.typo3.org/m/typo3/reference-typoscript/12.4/en-us/Setup/Config/Index.html#contentobjectexceptionhandler).

This means, this is already working:

10 = FLUIDTEMPLATE
10 {
  # Don't let exceptions break out of template, use default handling
  contentObjectExceptionHandler = 1
  contentObjectExceptionHandler.errorMessage = This Fluid Template is broken
  # use custom handling
  contentObjectExceptionHandler = Vendor\Namespace\ExceptionHandler\MyExceptionHandler
}

This should then also be added to other parts where a fluid view object is instantiated, like:

This would currently not work, as plugin.tx_myextension_pi1 is currently not directly bound to any
content object. Nevertheless, every Extbase plugin currently is bound to an content object of type EXTBASEPLUGIN,
which could be configured likewise already.

The primary goal would be to limit exceptions to the template and would not affect other parts of the page.

That is what currently already happens by default in production for every FLUIDTEMPLATE cObj, as well as for all Extbase plugins.

I think that both could also co-exist.

That is why I proposed one additional example, that would allow to gracefully fall back to default content for partials or parts of templates more generally speaking.

Thanks for deep diving in Helmut. Just to ask/clarify:
The FLUIDTEMPLATE contentObjectExceptionHandler will not throw an Exception, but the whole Template-Rendering will still be broken or to say it more clear: There will be no exception but the rendering isn't done and there's only an error-message instead of the rendering only "missing" the broken ViewHelpers?

I still don't think that's a practical way to solve the initial problem I've been trying to discuss and solve, that a missing "image" isn't worth breaking a whole page. Never the less it's done by an Exception (or HTTP 500) nor by replacing the Template that may contain broken images with an Error-Message (instead of the page-rendering missing only images).

I hope you understand what I mean? I don't just want to get a "user-friendly exception", I want a page rendering that still works. Or better - the developer should be able to catch these errors in a central place (not in the template) to decide which "broken" ViewHelper may be a reason to chancel rendering or just leave it off the rendering.

@Helmut Hummel Oh wow, I clearly misunderstood the documentation there. I thought that this is only a global option.

In fact, I think that your example wouldn't work (because of contentObjectExceptionHandler instead of exceptionHandler), while my hypothetical one above would in fact work right now. That is if the example code in the documentation is correct. Crazy. :)

And would that approach let the rest of the template render or would it break the whole rendering then?

This is a "safeguard", if you will, to not leak exceptions to other content elements. But rendering will stop at this point, as the exceptions bubble up to the content element.

For more fine-grained control I agree with @Helmut Hummel in #101928-9. We can't determine for every situation that it is "ok" to continue with rendering if an exception occurred. This needs to be dealt with in the template, one way or the other.

Ok I understand that. So then the only possible solution would be to do this directly in some fluid/core ViewHelpers, at least for those dealing with FAL (like f:image).

Just to remember, initially I was just wondering why the core ViewHelpers are declared "final class" now. I mean this itself is okay, but the whole code/logic is still there and not available for usage in customized ViewHelpers.

I've simply extended some of these classes in earlier TYPO3-Versions and implemented my try/catch logic around "parent::render" or "parent::renderStatic" to archive what we are now discussing here.

This isn't possible anymore! I've noticed some Core ViewHelpers extend their respective "parents" in the Namespace "TYPO3Fluid\Fluid\ViewHelpers" where are still classes that are non-final. If the final-classes would have implemented like that keeping an "door open" for developers to reuse the logic in their own ViewHelpers, we wouldn't discuss this here!

I also had a ticket about discussing the reasons for the "final classes" without implementing the code-logic inside into an accessible (extendable) class or Trait which then would be used in the final class (which in fact only implements the code-logic and final'izes the class).

But with this approach one has to re-implement the whole code-logic on oneself (or copy the original class), which leads to broken updates from core-updates as the whole ViewHelper is then on it's own without link to the original core-class.

I also analysed the inital ticket/task that was obviously the reason to implement those final classes. But I didn't find any task/ticket or discussion about pro's and con's nor any reasons. It was just "a task" that had been done without discussing this with the community or developers using TYPO3 in their daily-business being now affronted with the result that they have to live with exeptions that had already been avoided earlier.

I wish there would a generic core-solution for this! But if this isn't possible I would at least want to be able to do so on my own (and I'm shure other project developers using TYPO3 would agree).