PrivateTempStore->getOwner Attempts to access possibly unset Request Session

getting this same error as well when trying to update the address module. This error prevents drush updb -y from completing the address module's db updates. I took a stab at a patch for this.

Hello I'm Michael,
I've seen that this method is using the chaining methods without checking if the result, which is being returned, is either a null or something else.

Therefore, I have created this patch to solve this issue.

Thank you for the patch. Here are a few changes:
1. I believe we can safely chain getCurrentRequest()->getSession(), because we should always have a request, so we do not need to check the result of every function.
2. Avoiding the error however may cause another problem - freely getting stored objects with owner NULL. This can be fixed either in get() not to return the object, or in set() so that it's not stored at all.
3. PrivateTempStore has been moved from Drupal\user to Drupal\Core\TempStore.

Worked well for me. I faced this error when running configuration migrations. Here is an screenshot:

I am also uploading another patch where I have updated the phpdoc and made explicit the return value.

Doh. Removed an unneeded blank line.

Let's see if that has fewer errors.

More semantic use of the API.

@heddn, I'm a bit fussy but $has_session implies a boolean value is stored. There is a hasSession() method that does return a boolean on that object though.

Edit, apparently my coffee didn't kick in yet...

+++ b/core/lib/Drupal/Core/TempStore/PrivateTempStore.php
@@ -219,7 +207,34 @@ protected function createkey($key) {
+  protected function startSession() {
+    $has_session = $this->requestStack
+      ->getCurrentRequest()
+      ->hasSession();
+    if (!$has_session) {

That's what I'm doing, no?

I was until now using the work-around patch from #2976862: PrivateTempStore throws exception if session is not started (drush users) which works spendidly. But tht's just aworkaround, so I thought I would check out this one. Currently testing, it applied without problems and I don't get errors anymore. I have used it with half a dozen custom migration files so far.

Edit:

One question though? Why is this still (again) on NW ?
Ah a failure on testing:

There was 1 error:
1) Drupal\Tests\node\Functional\NodePreviewAnonymousTest::testAnonymousPagePreview
Behat\Mink\Exception\ExpectationException: Link with label Back to content editing found.

/var/www/html/core/tests/Drupal/Tests/WebAssert.php:437
/var/www/html/core/tests/Drupal/Tests/WebAssert.php:231
/var/www/html/core/modules/node/tests/src/Functional/NodePreviewAnonymousTest.php:57

Unfortunately I have no idea how to fix that.

This should fix the test.

I don't see the interdiff included in the patch? Is it just me?

I don't see the interdiff included in the patch? Is it just me?

The problem was that the patch in #12 removed some code that was still needed. Just the extra $this->startSession(); was added to PrivateTempStore::set().

sorry if I disturb the thread with a backport, but I need an equivalent patch for an old 8.4.x and the CI

reset need review for the comment #20 #2860341-20: PrivateTempStore->getOwner Attempts to access possibly unset Request Session

Patch from #20 works fine for me. Applies, fixes my problem (which was that importers weren't running with migrate_tools ), saw no other error. Been using it for 3 days without problems.

@robertom the last patch on the issue should be the rtbc patch so adding an 8.4.x does upset the issue. If you do this in the future please upload the correct patch again in the next comment.

Updating commit credit to credit people who contributed patches which progressed the issue.

Committed and pushed 7ae9670d22 to 8.7.x and 590a5e7904 to 8.6.x. Thanks!