Problem/Motivation
We have a hook_rebuild that calls Url::fromRoute()->toString().
We calling drush cr, it works correctly. But when calling drush updb, it crashes with this error:
TypeError: SplObjectStorage::offsetExists(): Argument #1 ($object) must be of type object, null given in /var/www/web/core/lib/Drupal/Core/Render/Renderer.php on line 655 #0 /var/www/web/core/lib/Drupal/Core/Render/Renderer.php(655): SplObjectStorage->offsetExists(NULL)
#1 /var/www/web/core/lib/Drupal/Core/Render/Renderer.php(626): Drupal\Core\Render\Renderer->getCurrentRenderContext()
#2 /var/www/web/core/lib/Drupal/Core/Render/MetadataBubblingUrlGenerator.php(87): Drupal\Core\Render\Renderer->hasRenderContext()
#3 /var/www/web/core/lib/Drupal/Core/Render/MetadataBubblingUrlGenerator.php(110): Drupal\Core\Render\MetadataBubblingUrlGenerator->bubble(Object(Drupal\Core\GeneratedUrl), Array)
Steps to reproduce
Call something like this in a context where there is no current request:
// The route needs to exist.
Url::fromRoute('sitemap.page')->toString();
Proposed resolution
Renderer::getCurrentRenderContext() should check if $request is null before using it.
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3497935
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3497935-renderergetcurrentrendercontext-triggers-a
changes, plain diff MR !10835
Comments
Comment #2
prudloff commentedComment #4
prudloff commentedI just noticed that I did not have the error on another website.
Turns out Drush usually provides a current request but I was using this patch that somehow removed the request from the request stack (I am not sure how honestly).
I am keep the MR open because it might still be worth it to make
Renderer::getCurrentRenderContext()more robust when there is no request.Comment #5
quietone commentedChanges are made on on 11.x (our main development branch) first, and are then back ported as needed according to the Core change policies.
@prudloff, in the future can you leave all headings in the issue summary. They are used to keep track of progress.
Comment #6
prudloff commentedComment #7
smustgrave commentedThank you for including test coverage! Makes it super easy to review
Left 2 small comments on the MR
If you are another contributor eager to jump in, please allow the previous poster at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!
Comment #8
prudloff commentedComment #9
smustgrave commentedFeedback appears to be addressed
Comment #14
catchCommitted/pushed to 11.x and cherry-picked to 11.1.x, 10.5.x, 10.4.x, thanks!