Fatal error: Call to a member function getOption() after creating a new page

Hm, under what circumstances does this happen?

Oh, I guess this is only triggered together with #2284005: Implement static contexts, without that, the context is not requested. But the tests there are not failing.

This happens also on head. (Perhaps this has changed since #3 was written.)

I think this goes unnoticed in development environments because the route rebuilding has time to happen before the redirect request i handled. For one D8 project, we have 700 routes which are all rebuilt after creating a new page. Depending on the server, this will take longer than handling the redirect request. Then in PageEditForm::form() -> getContexts() -> ... -> RouteParamContext::onPageContext(), getRoutesByPattern() does not find the newly created route.

One way to trigger the same error in development is to place a breakpoint and stop execution in RouteParamContext::onPageContext() PageManagerRoutes::alterRoutes(). Drupal should then proceed to handle the new request but without the new route registered.

Update: Fixed method reference.

Arguably, this is a design mistake in how Core handles redirects. Generally this problem would occur when a form, which creates a new route upon submit, redirects to a page that depends on the existence of the new route. I don't know if there are more rebuilding mechanisms, other than that of routes, that have the same problem. What we can do in contrib is to tolerate that such registries (like the router table) are not necessarily up to date—especially after a redirect.

Perhaps we should add a message to the current patch, to explain the silly situation that the user might have to refresh the page to see all contexts.

Sorry for the post spree.

Updated quickfix with a message. Not sure how to test this.

I was able to reproduce this on my local dev in a fresh install of D8 (beta15):

1. Install clean D8 (beta15)
2. Download -dev versions of page_manager, ctools, layout_plugin, panels
3. drush en -y panels
4. Go to Structure -> Pages and create a new page: "Test Page", path: testpage
5. When redirecting to /admin/structure/page_manager/manage/testpage I got the getOption() error in the original post
6. Refreshing the page shows the form without any error
7. Go to Structure -> Pages and create a new test page. Get same error on redirect.

So, it looks like this happens consistently the first time creating a new page. This is on my docker container on my Macbook, so its not that slow, so not sure about the timing issues mentioned earlier.

I agree with #6 about what is happening here, but I don't see any core issue for this. I'm not sure the watchdog message in #7 is needed. The patch in #1 seems to fix the problem for simple pages but I haven't tested anything with context.

Hmm, I take back my comment on the timing. As soon as I started trying to debug this with xdebug and phpstorm it no longer gets an error.

Edited: In fact, after clearing the database and reinstalling I still don't get the error anymore. So I cannot confirm whether the patch works or not because it's possible the timing changed. I'll continue to try and find a way to reproduce this consistently.

It'd be nice to be able to reproduce this, but I'm guessing this is a suitable fix.

Forgive me for any first-timer ignorance on some of this, but I've been tracing through the code to see how this might be happening. Here is what I've learned in case it helps trigger somebody's brains:

• When a new page is added, the alterRoutes() function in PageManagerRoutes is called, which loops through all Pages and creates a new Route object for each. NOTE: Seems like a potential performance issue to recreate *all* these routes every time, but see more below.
• The Routes collection is marked as rebuildNeeded in the page entity postSave() function.
• The Core RouteBuilder calls rebuild() if rebuildNeeded is set and is called during the Drupal onKernelTerminate() handler. This rebuild() function calls the dump() function that actually causes the "router" DB table to be updated. NOTE: Core actually deletes the "router" table and then inserts ALL the routes again. e.g., it is literally rebuilding the entire route table. Again seems like a performance issue that there isn't a clean way in core to just update a route or add a route without recreating anything.
• The onPageContext() is called to add the user entity to the context of the page. This loads all routes using a direct connection->query in the core RouteProvider service getRoutesByPath() function.

So, there are a couple possibilities here:

1. Maybe the onKernelTerminate() is not getting called sometimes so the new route is not dumped to the database yet. Seems unlikely that a simple page refresh would fix this. But I remember some issues in D7 where kernel termination was sometimes an issue.
2. RouteBuilder is saving the routes to the DB by starting a Transaction where it first deletes the entire Router table, then uses connection->insert to insert all the known routes. There is an exception handler at the end that does a transaction->rollback if there was any error. So anything causing an exception during this routine would prevent the route from being saved. Again, the fact a page refresh fixes it means this probably isn't the cause.
3. The RouteBuilder starts the transaction, but doesn't actually call Commit. It relies on something else in Drupal to eventually commit the transaction. Not sure where this commit occurs and if that's related.
4. Assuming the data really is written to the DB, the next possibility is that there is a DB cache somewhere such that the connection->query() to load the routes in onPageContext() is getting the previous table values rather than the recently inserted values.

In patch #1 it is assuming that the $routes array being returned is empty. Like it's not in the database or the database result is cached somewhere. The patch in #11 is a bit more general and just checks to ensure there is the correct Route object type in the return. This also handles the case of an empty $routes array, so seems like a cleaner solution.

But neither address the actual underlying problem. What is causing the DB query to not return the path that was just added. This seems like a more serious, potentially core issue with the DB caching or something.

Sorry for the long post, but are people aware of any DB caching that might be preventing the previous transaction results from appearing in the new db query?

Related to #3, I'm not seeing a specific call to the connection->commit anywhere, so somehow it is relying on the database to "lazy commit" this dangling transaction. Perhaps in some cases we are trying to query the DB before this transaction has been committed.

Can somebody more expert in D8 describe why the Drupal/Core/Routing/MatcherDumper.php dump() routine doesn't explicitly commit the transaction it has started? or how this "lazy commit" stuff is supposed to work?

D8 transactions works the same as they do in D7, that's nothing new. A transaction is committed basically at the end of a method.

Discussed this with @dawehner, who pointed us to #2564921: In PHP-FPM environment, enabling a module in using a 'configure' route leads to an error page.
Turns out @mpotter uses PHP-FPM and I do not, which explains why I could not reproduce.

Page Manager can still be broken with the fix in the last patch, but it's better than nothing. The other issue should fix it properly.

Working on a unit test now.

is this a duplicate of #2564921: In PHP-FPM environment, enabling a module in using a 'configure' route leads to an error page?

@valthebald Yes, that issue should remove all of the ways to make $route instanceof Route be FALSE, but I think it's a good improvement. I'll commit it shortly.

Apparently I committed this and didn't include the issue number: http://cgit.drupalcode.org/page_manager/commit/?h=61e8602