3.1 Issues with TranslationManager defaultLanguage after GraphQLLanguageContext::executeInLanguageContext

[megadesk3000]

Hello together.
We recently updated one of our Projects to use Version 3.1 instead of 3.0-rc3 before. We use it together with graphql_twig. So we are using the hybrid approach and not the fully decoupled one.

After that update we have really strange issues with interface translation on the site.

Example:

1. We have a node in german (page's default language) and english
2. If we view the node in german, all interface translation used in the template of the node are outputted correct in german.
3. If we view the same node in english, the node itself is correctly displayed in english BUT "most" interface translations on the page are still in german.

We had a really hard time debugging this issue, but in the end it turned out, that code in GraphQLLanguageContext::executeInLanguageContext, introduced in 3.0 makes the interface translations fail since we updated.

I will try to explain, what we think, the issue is:

public function executeInLanguageContext(callable $callable, $language) {
    $this->languageStack->push($this->currentLanguage);
    $this->currentLanguage = $language;
    $this->isActive = TRUE;
    $this->languageManager->reset();
    // This is needed to be able to use the string translation with the
    // requested language.
    $this->translationManager->setDefaultLangcode($language);
    // Override the configuration language so that config entities (like menus)
    // are loaded using the proper translation.
    $currentConfigLanguage = $this->languageManager->getConfigOverrideLanguage();
    if ($currentConfigLanguage->getId() !== $language) {
      $configLanguage = $this->languageManager->getLanguage($language);
      $this->languageManager->setConfigOverrideLanguage($configLanguage);
    }
    // Extract the result array.
    try {
      return call_user_func($callable);
    }
    catch (\Exception $exc) {
      throw $exc;
    }
    finally {
      // In any case, set the language context back to null.
      $this->currentLanguage = $this->languageStack->pop();
      $this->isActive = FALSE;
      $this->languageManager->reset();
      // Restore the languages for the translation and language managers.
      $defaultLangcode = !empty($this->currentLanguage)
        ? $this->currentLanguage
        : $this->languageManager->getDefaultLanguage()->getId();
      $this->translationManager->setDefaultLangcode($defaultLangcode);
      $this->languageManager->setConfigOverrideLanguage($currentConfigLanguage);
    }
}

Note that $this->translationManager->defaultLangcode is en when entering executeInLanguageContext, which is correct while displaying an english node.
The first thing that is done, is pushing $this->currentLanguage to the languageStack: $this->languageStack->push($this->currentLanguage);
While debugging that, it turned out that $this->currentLanguage was always NULL at this point. Maybe there are cases, where this is not the case, but i have not seen any other value than NULL here.
$this->currentLanguage = $language; was set with the correct language, passed in from outside, in case of the english node "en" is passed here.
Now en is set as the default language of the TranslationManager: $this->translationManager->setDefaultLangcode($language);. This is also ok.

The problem that we see, is coming from the finally block:
$this->currentLanguage = $this->languageStack->pop(); pops the NULL we pushed to languageStack before.
The next line makes the troubles:

// Restore the languages for the translation and language managers.
$defaultLangcode = !empty($this->currentLanguage)
        ? $this->currentLanguage
        : $this->languageManager->getDefaultLanguage()->getId();

I assume what we want to do here is to reset, the defaultLangcode to what it was before we entered the method. but instead we always set it to the default langcode of the page, since $this->currentLanguageis always null and therefore $this->languageManager->getDefaultLanguage()->getId() is used.

The next line now set this "wrong" langcode back to the TranslationManager and we end up with de set in the TranslationManager defaultLangcode.

All Interface translations that are performed outside of the renderInLanguageContext are now outputted in the default language of the website instead of the current language.

We wrote a little patch to fix our Issue and for us that looks ok. But we would be happy to get some feedback on that, since i don't know if that is only an issue in the hybrid decoupled approach, or maybe there is something else going wrong here.

I opened a PR: #1183