FluidTYPO3 / flux

TYPO3 extension Flux: Dynamic Fluid FlexForms
https://fluidtypo3.org
146 stars 212 forks source link

Uncaught exception for removed files #117

Closed bjo3rnf closed 11 years ago

bjo3rnf commented 11 years ago

We have some FCEs with <flux:flexform.field.file /> fields. When an existing file has been assigned to such a field and the file is deleted for some reason an exception is thrown:

TYPO3\CMS\Core\Resource\Exception\FileDoesNotExistException thrown in file
/Users/foo/www/typo3_src-6.0.4/typo3/sysext/core/Classes/Resource/Driver/AbstractDriver.php in line 399.

This exception should be caught or not thrown at all to avoid WTF.

NamelessCoder commented 11 years ago

Agree. And might I add: damn you, FAL, for throwing an exception instead of returning NULL in so many unnecessary cases.

bjo3rnf commented 11 years ago

Ah, FAL again. Anything you/we can do here on our side?

NamelessCoder commented 11 years ago

Anything you/we can do here on our side?

Let's find out :)

bjo3rnf commented 11 years ago

Moving files via fileadmin leads to the same exception. I thought that's what FAL is all about...?

NamelessCoder commented 11 years ago

I thought that's what FAL is all about...?

Beats me. The most I've gotten out of it so far is new Exception types in new and exciting places.

bjo3rnf commented 11 years ago

This particular exception is thrown after getSingleField_typeFlex_draw() is invoked in the override class. Could be caught there of course, but what then? And do you think it is worth the effort to check which exceptions might be raised as well to create a plan? Questions for the weekend ;)

NamelessCoder commented 11 years ago

I think the fix will probably confuse a bit, but it's certainly better than an uncaught exception. We can wrap \TYPO3\CMS\Backend\Form\FormEngine::getSingleField_SW and catch this particular type of exception, if caught we can attempt to fix the field value before attempting re-render.

Ugly.

NamelessCoder commented 11 years ago

Does the Exception by any chance contain a property with the name of the file...?

bjo3rnf commented 11 years ago

Only in the exception trace. It's a dumb exception :\

NamelessCoder commented 11 years ago

I think I have a solution for it - but it's not pretty; it removes the entire value if a FileNotFoundException is encountered. It's better than the Exception, but still not a very friendly thing to do. I was hoping for a way to detect the individual file that's failing and only remove it :/

NamelessCoder commented 11 years ago

How is this one treating you, Björn? Too drastic...?

bjo3rnf commented 11 years ago

The current behavior without your patch is too drastic =) I think this is an edge case and FluidTYPO3 can't save the world so I'm fine with this.

NamelessCoder commented 11 years ago

Perhaps one last thing. Adding a FlashMessage when this happens, including a dump of the old field value.

bjo3rnf commented 11 years ago

Sure, even better. In the end it's a user's fault that raises the exception but this way she knows why.

NamelessCoder commented 11 years ago

In the end it's a user's fault

I actually blame FAL - those references should be stored and warned about when removing FAL files; they are in other contexts. But then again, this reference being inside a FlexForm does relieve them of a small part of that blame :)

NamelessCoder commented 11 years ago

Closing this one - solved by the merged PR :)