Node add gives a Call to a member function getAccountName() on null Drupal 9

Hi, I added some more checks

+    // Set default author.
+    $meta_author = \Drupal::currentUser()->getAccountName();
+    if (!$node->isNew() && $node->getOwner() != null) {
+      $meta_author = $node->getOwner()->getAccountName();
+    }

Hitting this with D11 , client deleted a user

Suggested workarounds from comment #8 and related suggestions did not yield any clues. Patch 32 works for us. Our client deleted some users in prod that were involved in site building and some specific use cases involving content. Result is a WSOD for related environments when doing a node edit. The page in question has nothing special, just a node form with some normal fields. The reference to the user that is null is somewhere in a revision or as author/owner. This is about all the meta I can provide at this time.

> changed target branch from 11.x to 11.1.x

Should be 11.x

The issue title says node/add but that looks like a very old config export that was fixed in a different issue.

It doesn't make sense to fall back to the current user. If there is no current owner, it shouldn't display anything. it will afaik set the current user then as owner on save, but it's not yet true. So it should just not display anything then.

This _should_ not happen, deleting users should either delete the content or reassign it, this is just about dealing with invalid data.

And yes, originally the issue was about problems with the default value config that wasn't updated, I think a new issue would have made more sense, but it's technically speaking that error.

It doesn't make sense to fall back to the current user. If there is no current owner, it shouldn't display anything. it will afaik set the current user then as owner on save, but it's not yet true. So it should just not display anything then.

Yes agreed with this, and should possibly trigger_error() an E_USER_WARNING since it could be the result of a botched database update or attempt to prune user accounts and similar.

While it shouldn't happen, it did for us and it is 100% content or revision related. We have about 6 environments of the same build. My copy is a production dump from before the client deleted users. In my copy, I do not see the exception however in the more recent production copy where the users were deleted there is an exception and the patch is needed in this case.

So 100% the configuration is the same in our test cases however the content is not the same.

acbramley → closed merge request !11417

acbramley → changed the visibility of the branch 3161212-node-add-gives to hidden.

Working on this, closing previous MR as the branch can't be rebased easily and it was against 11.1.x

It's actually quite difficult to contrive a scenario with core to hit a node add/edit page with a NULL author.

The easiest way for node/add is to override the default value callback to stop setting it to the current user.

I tried to get edit form coverage as well but as soon as you delete the user, the node is gone too because of NodeHooks1::userPredelete

The uid column is non-NULL so we can't update the db directly.

As for wording when the user is missing, I think we can just accept an empty string here hopefully? Seeing as this is quite a rare scenario I think as long as we don't WSOD that's good enough. The author will be set on next save anyway.

What #49 said (and what I said in #41). This shouldn't happen, only thing I can think if is doing things directly in the database and bypassing entity API. (#49 is not quite correcting that regard. You don't need to set it to NULL, that's not what the scenario is about. It's about either setting it to a non-existing UID or deleting said user record without triggering entity hooks).

But I'm not against hardening our code against invalid data. I probably wouldn't even have asked for test coverage but great that you added it anyway.

You don't need to set it to NULL, that's not what the scenario is about. It's about either setting it to a non-existing UID

Tried that and even that's not possible, we have this in Node::preSave

      // If no owner has been set explicitly, make the anonymous user the owner.
      if (!$translation->getOwner()) {
        $translation->setOwnerId(0);
      }

But yeah I agree, this is more than enough :)

Ran test-only feature

1) Drupal\Tests\node\Functional\NodeEditFormTest::testNodeFormNullAuthor
Behat\Mink\Exception\ExpectationException: Current response status code is 500, but 200 expected.
/builds/issue/drupal-3161212/vendor/behat/mink/src/WebAssert.php:888
/builds/issue/drupal-3161212/vendor/behat/mink/src/WebAssert.php:145
/builds/issue/drupal-3161212/core/modules/node/tests/src/Functional/NodeEditFormTest.php:267
FAILURES!
Tests: 4, Assertions: 105, Failures: 1.

99% of the code is tests so the actual fix makes sense.

Very nice.

Latest MR looks good, no attempt to replace it with something else, just avoid the fatal error.

Committed/pushed to 11.x and cherry-picked to 11.1.x, thanks!

This is eligible for backport to 10.5.x, but we've broken 10.5.x several times recently cherry-picking 11.x commits without a full test run, so marking fixed. If you'd like this to be backported to 10.5.x, feel free to re-open with a 10.5.x-specific MR - it might just need a cherry-pick, but that way we get a full test run before commit.

catch → closed merge request !11763

Automatically closed - issue fixed for 2 weeks with no activity.