flux icon indicating copy to clipboard operation
flux copied to clipboard

FCE throwing Exception "Call to a member function getGrid() on null"

Open typoworx-de opened this issue 5 years ago • 6 comments

After a flux migration to recent version (9.x) there is an FCE-Element that is throwing "Call to a member function getGrid() on null". I've already migrated all Templates (Page & FCE) to latest "flux" standards as good as known and documentated.

The Error is thrown because $record is returned NULL in Content/GetViewHelper on Line ~130.

The FCE-Elemement looks fine in backend at all. There's nothing "obviously" total wrong which could lead to this issue.

I still understand the reason for this issue. And on the other hand such errors should be avoided at least in Production-Environment. It's not perfect, but a lot better to return no FCE-Content-Element but trying to deliver the rest of the page than causing the whole page being destroyed by such error.

Well I know this is just a suggest (which should be taken to a new ticket at all), but: In Development it would be greatful to have some kind of Error-Hinting by pointing out the Template-File and ideally what exactly could cause some kind of error -- like the given one. Even if such help will not point out always 100% perfect, any more precise error will help to avoid such problems either after migration or building new templates.

Any help what problem in record or template is causing this problem is welcome.

typoworx-de avatar Jul 20 '20 18:07 typoworx-de

Because I'm some kind of busy to get the customer-project done I'm doing some research writing it down here in hope it leads to find the culprit all together and maybe bugfix it in fluid as if required.

I debugged after Line ~130 in GetViewHelper adding this:

DebuggerUtility::var_dump($record, '$record');
DebuggerUtility::var_dump($renderingContext);

And surprisingly I noticed that $renderingContext -> variableProvider -> variables -> array-item 'record' exists and has valid array-data.

Trying to access it using this code also works!

$renderingContext->getVariableProvider()->get('record')

So why does the original code use php $renderingContext->getViewHelperVariableContainer() instead of the line above?

Is this some new approach which could break old FCE Records having improperly Flexform or Template or what is going on there?

typoworx-de avatar Jul 20 '20 18:07 typoworx-de

Trying to fetch the NULL'ed $record from getVariableProvider() instead works, but still does not solve the problem with 'Call to a member function getGrid() on null'.

Just for your notice I've been trying to fix it like this assuming the recent flux FCE work fine with etViewHelperVariableContainer:

On Line 134 replacing:

        if($renderingContext->getViewHelperVariableContainer()->exists(FormViewHelper::class, 'record'))
        {
            $record = (array)$renderingContext->getViewHelperVariableContainer()->get(FormViewHelper::class, 'record');
        }
        else if($renderingContext->getVariableProvider()->exists('record'))
        {
            $record = (array)$renderingContext->getVariableProvider()->get('record');
        }
        else
        {
            // Log this somewhere!
            $logger = GeneralUtility::makeInstance(LogManager::class)->getLogger(__CLASS__);
            $logger->error('Cannot get "record" from RenderingContext', [ 'File' => __FILE__, 'Line' => __LINE__ ]);

            if(!GeneralUtility::getApplicationContext()->isProduction())
            {
                throw new ContentRenderingException('Cannot get "$record" from RenderingContext');
            }

            return '';
        }

and same for $grid = .... on Line 141 to avoid breaking whole site at least in Production-Context

        // START BUGFIX | https://github.com/FluidTYPO3/flux/issues/1811
        try
        {
            if ($renderingContext->getViewHelperVariableContainer()->exists(FormViewHelper::class, 'provider'))
            {
                $grid = $renderingContext->getViewHelperVariableContainer()->get(FormViewHelper::class, 'provider')->getGrid($record);
            }
        }
        catch (\Throwable $e)
        {
            throw $e;
        }
        finally
        {
            if($grid === null)
            {
                $logger = GeneralUtility::makeInstance(LogManager::class)->getLogger(__CLASS__);
                $logger->error('Cannot get "provider" from RenderingContext', [ 'File' => __FILE__, 'Line' => __LINE__ ]);

                if (!GeneralUtility::getApplicationContext()->isProduction())
                {
                    throw new ContentRenderingException('Cannot get "provider" from RenderingContext');
                }

                return '';
            }
        }
        // END BUGFIX

typoworx-de avatar Jul 20 '20 18:07 typoworx-de

Just as an Idea if you consider using such approach. Make a local extended Class wrapping getVariableProvider and getViewHelperVariableContainer catching such exceptions and throwing the logging and/or error from the get-method key. This would spare a lot of code having this in a central place.

typoworx-de avatar Jul 20 '20 19:07 typoworx-de

Does anyone know how to solve the initial problem "Call to a member function getGrid() on null"? Other Flux FCE work fine. So I think it must be something in template or something 'special' in this kind of FCE leading to the exception.

typoworx-de avatar Jul 20 '20 19:07 typoworx-de

Okay... I think I've found the culprit by try & error. The frontend-section of flux-template had this in it, which was implemented by some recent developer before migration of the project:

The VHS render.uncache ViewHelper is deprecated (and obviously non-cached stuff always isn't good).

<v:render.uncache partial="TimeBox" arguments="{_all}" />

I still don't know why this stuff was leading to the given error above. I would have expected some kind of exception like "Unable to resolve ViewHelper "v:render.unache" (of course also in non-production only as exception).

I really would love if one of my suggestions above to catch errors will be used in further versions helping to avoid exceptions in Production and make this extension more solid.

typoworx-de avatar Jul 20 '20 19:07 typoworx-de

I'll give you some ideas about solving / working around this problem and then try to answer the many questions that came up.

First you have to know how the rendering process works:

  • Flux is asked to render a certain content type
  • Flux then resolves the Provider for the content type and reads which template file to render
  • Flux begins rendering the template file and assigns in ViewHelperVariableContainer an assortment of variables that may be required for rendering (the record, the form, the extension key, etc.)
  • Flux then renders the template to produce the output

The reason why this particular setup that you have (mixing v:render.uncache with flux:content.get) doesn't work is as follows:

  • TYPO3 itself has made changes to how objects are unserialized (goal: to avoid unsafe unserialization).
  • VHS v:render.uncache serializes the context (including the ViewHelperVariableContainer) in order to pass it to the detached and uncached USER_INT that handles the delegated rendering.
  • But because of the changes to TYPO3 the context is incomplete when unserialized, causing the problem you observed.

So avoiding it could be done in a couple of different ways:

  • If there's no good reason why the output should be uncached, remove the usage of v:render.uncache.
  • If it has to be uncached for some reason, but the sub-rendered content elements are okay to cache, use flux:content.get outside of v:render.uncache and assign the output records to a variable that you can then pass to v:render.uncache (they will be serialized and will be the same for all sub-renderings of the partial given to v:render.uncache).
  • If you must resolve the records inside the uncached partial, consider creating a TS object of type COA_INT and render with it, a FLUIDTEMPLATE object which has variables assigned, and make one of the variables a CONTENT or RECORDS object which selects your content records using a condition that's constructed using variables from data - then use f:cObject to do the rendering and pass the parent record as data="" attribute.

I'm aware this situation with the serialization changes in TYPO3 is annoying as hell, but it is unfortunately not within my control to make it behave differently. See also https://github.com/FluidTYPO3/vhs/issues/1647.

Now about the other questions:

In Development it would be greatful to have some kind of Error-Hinting by pointing out the Template-File and ideally what exactly could cause some kind of error -- like the given one.

Ideally, yes - but Fluid 2.x isn't capable of knowing which file was being rendered when an error is raised. It can only know the specific file during the parsing step and the error you face happens in the rendering step. Inbetween those, the template file was compiled to a PHP class and is no longer associated with a specific file. It no longer has a file name, it has an identifier which can be predicted when Fluid is asked to resolve a file (resolved file shares identifier of compiled PHP class; Fluid can ask the cache if file with identifier XYZ is compiled and if it is, Fluid gets an instance of the compiled class instead of going through the parsing step).

Fluid 3.0 is a different story, it is fully aware of the file in all use cases except for when called with a template source string as input - e.g. when there really isn't any file.

So why does the original code use $renderingContext->getViewHelperVariableContainer() instead of the line above?

Because the ViewHelperVariableContainer isn't affected by any variables you assign in the template. If Flux only used the template variables, you would have to manually pass along the record variable, and substituting it or modifying it could cause things to break. Flux uses the ViewHelperVariableContainer for things that should work regardless of variable composition and which should always be present without requiring you to manually pass variables. And Flux manages the composition of this container when initializing the rendering.

...and this only causes a problem for your use case because the container is nulled, which in turn is caused by a seemingly unrelated change in TYPO3's serialization capabilities. Normally this wouldn't be any problem whatsoever, but you get affected by an edge case.

Just as an Idea if you consider using such approach. Make a local extended Class wrapping getVariableProvider and getViewHelperVariableContainer catching such exceptions and throwing the logging and/or error from the get-method key. This would spare a lot of code having this in a central place.

This wouldn't be sensible - attempting to read an undefined variable isn't considered an error case in Fluid (value NULL is implied). If this was changed it would be the equivalent of the difference between E_NOTICE on or off in PHP, and it would be doubtful if the increased error logs would actually be helpful at all, as it would report any access to undefined variables.

Couple of general things

Swallowing errors is generally not a good approach, even in production. Ask youself if a logged error stating Cannot get "provider" from RenderingContext and pointing to an arbitrary PHP file from Fluid (can't report the template file, can't report the TS object, and so on) would help you track down the problem in any way? The only thing that would be able to give you a hint would be the stack trace.

This is why in most cases in both Flux and VHS, errors are not swallowed - instead they are allowed to happen and handled-or-not depends entirely on your error handling settings in TYPO3 (where you can define one error handling for development context and another for production). If configured with an error handler for production but none for development, then on production you would see a general error page (which you can configure in much detail, for example creating a styled error page) and on development you would get the stack trace. Most importantly: everything would behave this way - not just whichever classes from Flux or VHS that implemented this if-context-then-catch-otherwise-rethrow type of logic. And it would not require huge amounts of try-catch handling throughout.

Even further, TYPO3 has a setting that catches such errors when they happen specifically in rendering a content element - config.contentObjectExceptionHandler. This would handle (as in: swallow, log and generically report) errors arising from content rendering, which would encompass nearly all of Flux and VHS' errors - with one notable exception being v:render.uncache because internally it creates a delegated USER_INT object that technically isn't content rendering. You are probably aware of this setting; in particular since it doesn't have a context-specific toggle and therefore by default also swallows exceptions on development context and has to be disabled to do any meaningful debugging.

I know we've talked before about error handling in Flux and we don't agree about some of it - so I'm hoping that the explanations above are enough to give you a good idea about why Flux and VHS generally opts not to catch and suppress exceptions. The short version is that if they did suppress exceptions this only makes it more difficult for you to fix the issues and circumvents standard error handling in TYPO3, thus hiding the fact that an error did occur and robbing you of the ability to show a custom error page, do special logging, etc.

NamelessCoder avatar Jul 21 '20 10:07 NamelessCoder

Closing as solved - some guards have been added to ensure that $record is never null, and more descriptive exceptions have been added to inform developers of various context-related problems.

NamelessCoder avatar Nov 07 '22 14:11 NamelessCoder