Although a bad pattern, I recently saw this in a pre-release for a contrib module.
If a module is defining a global constant like this (for example on the top of its module file):
define('MY_GLOBAL', t('Test this'));
...and the site in question is installed in something other than english, then locale will try to save this string for translation, and try to save some metadata about it, among them, the request URI where the string was first found. Long story short, it ends up here (core/modules/locale/src/LocaleLookup.php line 152:
$this->stringStorage->createString(array(
'source' => $offset,
'context' => $this->context,
'version' => \Drupal::VERSION,
))->addLocation('path', $this->requestStack->getCurrentRequest()->getRequestUri())->save();
Now, looking at the documentation for getCurrentRequest, it can return "Request|null". In this case, it returns null, and so getRequestUri causes "Fatal error: Call to a member function getRequestUri() on null".
Now, one might argue that the fault here is entirely at the module author, because they should not create a global constant like that. Fair enough. But one might also argue that Drupal should maybe not fail so loudly. In my opinion, we could add a check for the return value of getCurrentRequest, and if "null" is returned, we set a default value of "/" (for example). We might also show a warning in watchdog for example, so that module authors can be notified in some way.
Open to suggestions, and will provide the first patch suggestion in next post.
Steps to reproduce:
- Install Drupal in another language than English
- Enable a new module.
- Add a global constant with t() (for example with define('MY_GLOBAL', t('Test this'));) to that module.
- Try to refresh any page on the site.
This does not occur if you try to enable a module with that constant already defined, but it will happen if you import config that enables a module that has this constant defined.
Anyway, discussion started - patch coming up :)
