Regression: provisionDrupalAccount returning FALSE triggers error on account creation

So this patch technically fixes the issue but since $this->saveAccount() doesn't actually return anything and the next line if (!$this->account) { as far as I can tell will never actually evaluate to TRUE, so I'm not totally convinced that the error handling in place to catch and inform of the User Entity saving failing will work.

If the User Entity does fail to save \Drupal\Core\Entity\EntityStorageException will be thrown (see EntityInterface::save), which as far as I can tell means that this is redundant:

    $this->saveAccount();
    if (!$this->account) {
      drupal_set_message(t('User account creation failed because of system problems.'), 'error');
      return FALSE;
    }

So, I'm setting to needs review because technically speaking the user-facing issue is resolved, but in my opinion DrupalUserProcessor::createDrupalUser should either: always return TRUE beyond $this->saveAccount() and the if statement after it should just be removed, or wrap $this->saveAccount() in a try catch block and return FALSE if \Drupal\Core\Entity\EntityStorageException is thrown. However, I'm not at all a fan of catching exceptions like that, because if an exception is thrown in the Entity Storage you really need that to bubble up so that the administrator becomes aware of it. So in my opinion the end of DrupalUserProcessor::createDrupalUser really should just look like:

    $this->saveAccount();
    ExternalAuthenticationHelper::setUserIdentifier($this->account, $this->account->getAccountName());
    return TRUE;

Because, again, if something did do wrong at $this->saveAccount() the next line will never be reached because \Drupal\Core\Entity\EntityStorageException will be thrown.

@grahl great thanks for that