Form::getPage() throws Error when returning null

[pamtbaau]

When $form->getPage() is called in onFormInitialized in plugin, an Error is thrown, when page has not yet been initialized. When page is not yet cached, onFormInitialized is called before onPageInitialized and hence $form->getPage() cannot return a page.

• Shouldn't the signature of the function be nullable: public function getPage(): ?PageInterface?
• Or should event onFormInitialized perhaps always be fired after onPageInitialized?

Error thrown:

Return value of Grav\Plugin\Form\Form::getPage() must implement interface Grav\Common\Page\Interfaces\PageInterface, null returned

Stacktrace:

[2021-04-12 17:40:09] grav.CRITICAL: Return value of Grav\Plugin\Form\Form::getPage() must implement interface Grav\Common\Page\Interfaces\PageInterface, null returned -
Trace:
#0 /path/to/user/plugins/qq/qq.php(78): Grav\Plugin\Form\Form->getPage()
#1 /path/to/vendor/symfony/event-dispatcher/EventDispatcher.php(264): Grav\Plugin\QqPlugin->onFormInitialized()
#2 /path/to/vendor/symfony/event-dispatcher/EventDispatcher.php(239): Symfony\Component\EventDispatcher\EventDispatcher->doDispatch()
#3 /path/to/vendor/symfony/event-dispatcher/EventDispatcher.php(73): Symfony\Component\EventDispatcher\EventDispatcher->callListeners()
#4 /path/to/system/src/Grav/Common/Grav.php(556): Symfony\Component\EventDispatcher\EventDispatcher->dispatch()
#5 /path/to/user/plugins/form/classes/Form.php(203): Grav\Common\Grav->fireEvent()
#6 /path/to/user/plugins/form/classes/Form.php(175): Grav\Plugin\Form\Form->initialize()
#7 /path/to/user/plugins/form/classes/FormFactory.php(38): Grav\Plugin\Form\Form->__construct()
#8 /path/to/user/plugins/form/classes/Forms.php(79): Grav\Plugin\Form\FormFactory->createFormForPage()
#9 /path/to/user/plugins/form/form.php(197): Grav\Plugin\Form\Forms->createPageForm()
#10 /path/to/vendor/symfony/event-dispatcher/EventDispatcher.php(264): Grav\Plugin\FormPlugin->onPageProcessed()
#11 /path/to/vendor/symfony/event-dispatcher/EventDispatcher.php(239): Symfony\Component\EventDispatcher\EventDispatcher->doDispatch()
#12 /path/to/vendor/symfony/event-dispatcher/EventDispatcher.php(73): Symfony\Component\EventDispatcher\EventDispatcher->callListeners()
#13 /path/to/system/src/Grav/Common/Grav.php(556): Symfony\Component\EventDispatcher\EventDispatcher->dispatch()
#14 /path/to/system/src/Grav/Common/Page/Pages.php(1822): Grav\Common\Grav->fireEvent()
#15 /path/to/system/src/Grav/Common/Page/Pages.php(1841): Grav\Common\Page\Pages->recurse()
#16 /path/to/system/src/Grav/Common/Page/Pages.php(1841): Grav\Common\Page\Pages->recurse()
#17 /path/to/system/src/Grav/Common/Page/Pages.php(1701): Grav\Common\Page\Pages->recurse()
#18 /path/to/system/src/Grav/Common/Page/Pages.php(1690): Grav\Common\Page\Pages->resetPages()
#19 /path/to/system/src/Grav/Common/Page/Pages.php(1478): Grav\Common\Page\Pages->buildRegularPages()
#20 /path/to/system/src/Grav/Common/Page/Pages.php(315): Grav\Common\Page\Pages->buildPages()
#21 /path/to/system/src/Grav/Common/Processors/PagesProcessor.php(42): Grav\Common\Page\Pages->init()
#22 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(50): Grav\Common\Processors\PagesProcessor->process()
#23 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(62): Grav\Framework\RequestHandler\RequestHandler->handle()
#24 /path/to/system/src/Grav/Common/Processors/TwigProcessor.php(38): Grav\Framework\RequestHandler\RequestHandler->handle()
#25 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(50): Grav\Common\Processors\TwigProcessor->process()
#26 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(62): Grav\Framework\RequestHandler\RequestHandler->handle()
#27 /path/to/system/src/Grav/Common/Processors/AssetsProcessor.php(39): Grav\Framework\RequestHandler\RequestHandler->handle()
#28 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(50): Grav\Common\Processors\AssetsProcessor->process()
#29 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(62): Grav\Framework\RequestHandler\RequestHandler->handle()
#30 /path/to/system/src/Grav/Common/Processors/SchedulerProcessor.php(40): Grav\Framework\RequestHandler\RequestHandler->handle()
#31 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(50): Grav\Common\Processors\SchedulerProcessor->process()
#32 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(62): Grav\Framework\RequestHandler\RequestHandler->handle()
#33 /path/to/system/src/Grav/Common/Processors/BackupsProcessor.php(39): Grav\Framework\RequestHandler\RequestHandler->handle()
#34 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(50): Grav\Common\Processors\BackupsProcessor->process()
#35 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(62): Grav\Framework\RequestHandler\RequestHandler->handle()
#36 /path/to/system/src/Grav/Common/Processors/TasksProcessor.php(69): Grav\Framework\RequestHandler\RequestHandler->handle()
#37 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(50): Grav\Common\Processors\TasksProcessor->process()
#38 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(62): Grav\Framework\RequestHandler\RequestHandler->handle()
#39 /path/to/system/src/Grav/Common/Processors/RequestProcessor.php(63): Grav\Framework\RequestHandler\RequestHandler->handle()
#40 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(50): Grav\Common\Processors\RequestProcessor->process()
#41 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(62): Grav\Framework\RequestHandler\RequestHandler->handle()
#42 /path/to/system/src/Grav/Common/Processors/ThemesProcessor.php(38): Grav\Framework\RequestHandler\RequestHandler->handle()
#43 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(50): Grav\Common\Processors\ThemesProcessor->process()
#44 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(62): Grav\Framework\RequestHandler\RequestHandler->handle()
#45 /path/to/system/src/Grav/Common/Processors/PluginsProcessor.php(39): Grav\Framework\RequestHandler\RequestHandler->handle()
#46 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(50): Grav\Common\Processors\PluginsProcessor->process()
#47 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(62): Grav\Framework\RequestHandler\RequestHandler->handle()
#48 /path/to/system/src/Grav/Common/Processors/InitializeProcessor.php(130): Grav\Framework\RequestHandler\RequestHandler->handle()
#49 /path/to/system/src/Grav/Common/Debugger.php(546): Grav\Common\Processors\InitializeProcessor::Grav\Common\Processors{closure}()
#50 /path/to/system/src/Grav/Common/Processors/InitializeProcessor.php(131): Grav\Common\Debugger->profile()
#51 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(50): Grav\Common\Processors\InitializeProcessor->process()
#52 /path/to/system/src/Grav/Framework/RequestHandler/Traits/RequestHandlerTrait.php(62): Grav\Framework\RequestHandler\RequestHandler->handle()
#53 /path/to/system/src/Grav/Common/Grav.php(295): Grav\Framework\RequestHandler\RequestHandler->handle()
#54 /path/to/index.php(58): Grav\Common\Grav->process()
#55 {main} [] []

[mahagr]

This is expected and the behavior is correct. All the froms are created before a page gets initialized to be able to find the page where the form was located in. Forms are then cached. So there is no knowledge of which page you are on when the forms are created as the page keeps on changing but the form instances stay the same.

[pamtbaau]

Depending on whether a page has already been initialised and cached, onFormInitialized can be fired before or after onPageInitialized.

That means there are two scenarios:

1. Pages have not yet been initialised and Form::getPage() will not find a page
2. Pages have already been initialized and cached and Form::getPage() will return the page.

The discussion is whether scenario 1. is an unexpected/invalid state or not and should throw or return null.

• If unexpected: Throw an Error
  If this is how this state is interpreted by design, I would like to suggest to add the phpdoc * @throws TypeError if page is not found, to help IDE to warn the developer to use a try-catch.
• If expected: Return null and change return type.

[pamtbaau]

Thanks for the improvements!

I'm afraid however, that the new LogicException might be causing a breaking change, since the previous error was a TypeError. If any plugin uses a try-catch (\TypeError $err) it will no longer catch the error and break.

[mahagr]

TypeError is kinda restricted to PHP only. The correct is LogicError as it's a programming error to call it too early. I would not use the method at all in onFormInitialized event as you will likely run into caching issues if you do so.

[pamtbaau]

OK, fair enough...

Uhm... what would then be a better/save event to get the page and form before it is being send to the front-end? I'm not sure where in Grav's lifecycle fits the onFormInitialized fired by Forms plugin.

NB. Data from page's frontmatter is required to update the form definition.

[mahagr]

One place would be late in onPageProcessed event, which is used by the form plugin to make sure that forms have been initialized for the current page.

[pamtbaau]

Thanks for the suggestion, I appreciate it.

Unfortunately, adding extra fields to the form using Form::setFields() during event onPageProcessed doesn't seem to work. Maybe that's just too late in the lifecycle of the Form.

It does seem to work during onFormInitialized, using a try-catch. When Page has not yet been initialized, Form::getPage() will fail, but onFormInitialized is called again after Page has been initialized. During the second event, Form::getPage() succeeds and extra fields can be added.

public function onFormInitialized(Event $event) {
  $form = $event['form'];

  try {
    $page = $form->getPage();
    $header = $page->header();
    $events = $header->events;

    $this->addFormFields($form, $events);
  } catch (\TypeError $err) {
    // Ignore
  }
}

Events fired:

• Page not yet initialized:
  When Page has not yet been initialized, onFormInitialized is being called again after the page has been initialized:

  onFormInitialized: Page NOT available  (Event fired for each defined form)
  onPageInitialized
  onFormInitialized: Page available -> adding extra fields to form
  

• Page has been initialized:

  onPageInitialized
  onFormInitialized: Page available -> adding extra fields to form
  

Do you still expect issues using above code?

[mahagr]

Thanks for your testing on this. Maybe we need another event for this -- or some extra access to the form plugin to be able to manipulate the fields. I need to check the code again to verify if that works and if there are better ways to do this, but I do agree that the onPageProcessed event is too late.