How do I build a redirect on login function that doesn't halt hook execution

Frankly, I don't know if it happens otherwise. I've just upgraded to 9.2.0 now and couldn't log in any more. Checked the logs in Drush and saw the error message. First I suspected 9.2.0 itself but googling for the message lead me to the linked issue. Then I realized it was the redirect. I don't use any of those modules but have the same code in my own module, disabled that temporarily and lo and behold, I could log in once more.

The message was discussed earlier, too, in various issues (also linked from the linked issue). Suggestions include to follow the redirect with a

\Drupal::service('page_cache_kill_switch')->trigger();

or an

exit;

Neither seems to work here.

To give you some example, this is what my hook boils down to:

if (AccessResult::allowedIfHasPermission($account, 'whatever')) {
  $url = Url::fromRoute('<front>', [], ['absolute' => 'true'])->toString();
  $home = new RedirectResponse($url);
  $home->send();
}

Nothing fancy.

I found a verbose error message now:

RuntimeException: Failed to start the session because headers have already been sent by "/opt/drupal/vendor/symfony/http-foundation/Response.php" at line 368. üzenet Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage->start() függvényben (/opt/drupal/vendor/symfony/http-foundation/Session/Storage/NativeSessionStorage.php 152. sorában)
#0 /opt/drupal/web/core/lib/Drupal/Core/Session/SessionManager.php(162): Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage->start()
#1 /opt/drupal/web/core/lib/Drupal/Core/Session/SessionManager.php(193): Drupal\Core\Session\SessionManager->startNow()
#2 /opt/drupal/vendor/symfony/http-foundation/Session/Session.php(189): Drupal\Core\Session\SessionManager->save()
#3 /opt/drupal/web/core/lib/Drupal/Core/StackMiddleware/Session.php(60): Symfony\Component\HttpFoundation\Session\Session->save()
#4 /opt/drupal/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#5 /opt/drupal/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#6 /opt/drupal/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#7 /opt/drupal/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#8 /opt/drupal/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#9 /opt/drupal/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#10 /opt/drupal/web/core/lib/Drupal/Core/DrupalKernel.php(716): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#11 /opt/drupal/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#12 {main}

Yes, it does point to session.

I beg to differ. It's not the other module's fault. I just happened to find that issue queue by googling the error message because I use the same approach in my own code. Many modules and many custom code do that, it has always been a valid approach and even if you don't think that approach is the best, this is a breaking change introduced by a minor upgrade claiming perfect compatibility. What's worse, it leads to user #1 being immediately locked out of from many websites completely. This is not something that can be done.

I vehemently oppose turning this showstopper into a feature request. Please, revert it back immediately. Create an extra issue if you want to, trying to find a better solution and doing the "right thing", deprecating the old one, giving ample time for module authors and users to adapt. Not to mention that the functionality is required now in a myriad of sites, not in some distant future when a better solution will have been crafted,

But this is a cricital bug the has to be addressed somehow, and very quickly. You can't just say something that used to work for decades is no longer acceptable overnight. Especially as it leads to catastrophic consequences. Admins are caught unawares, might not all have the possibility to use Drush to uninstall a module without logging in, let alone knowing what locked them out in the first place. I don't even know how it could be handled for them, even if a very quick patch is rolled out, to move on if they can't log in any more.

It was bugging me and I need it for my own site, too, so I finally found the solution in the deprecation changelog of drupal_goto(): https://www.drupal.org/node/2023537

$response = new RedirectResponse($url->toString());
$request = \Drupal::request();
// Save the session so things like messages get saved.
$request->getSession()->save();
$response->prepare($request);
// Make sure to trigger kernel events.
\Drupal::service('kernel')->terminate($request, $response);
$response->send();

Note, larowlan, that according to the docs, this is the officially sanctioned way to do it. I'll quote this in the other queue as well, but my reasoning above still stands. This is a can of worms opened that can lead to very dire consequences for many admins unless something is done proactively, and I don't mean a support request.

Yes, I read it, I started to experiment but it doesn't work yet. First time I delve into middleware details, though...

I now start to see the bigger picture.

1. I don't think the argument "wrong because it halts execution, so we have to stop this dead in the tracks right now, whatever the cost" holds water. drupal_goto and what came after it have been used at least since D6 (probably earlier but I wouldn't know, that was the first version I started to work with). Login Toboggan and numerous other modules, plus probably tons of custom code of various developers used it like that since then, whether optimal and elegant or not.

2. Yes, the redirect module authors can be mildly chastised for not doing it the suggested way from the drupal_goto() changelog, namely saving the session and doing the two more housekeeping steps. Probably everybody copied the previous one, I also just copied my code from one of those modules, everybody did that. Adding those few lines seems to solve the problem (although in one case I kept receiving errors in the log but it started to work, I could log in, this is probably the best of both worlds because it shows there is something going on, while still allowing the normal operation to resume).

3. I accept the purist view of Larowlan that it should be done differently in the future, I also started to experiment with it in the other queue and got a working solution, I'm still waiting on expert opinion whether using the State API is acceptable in this case or not, but the concept is obviously proven, there is a way to go forward.

4. But that doesn't negate the fact that, whatever led to the current situation, it has to be dealt with somehow. Drupal surely has ways to address backwards compatibility issues, offering some interim, short term solution that saves people's bacon and then, when the problem has calmed down, we can go on fixing it the proper way.

My current take is in https://www.drupal.org/project/redirect_after_login/issues/3214949#comme....

I was partly subjected to the problem, although I don't use any of these redirect modules, I did use the same code in my own module. I spent the better part of today with this issue, first I was dismayed by Larowlan's approach, then I delved into what he suggested, then finally I found out that using this hook is really not necessary and people could have, should have moved on for quite some time (meaning years) now. And not necessarily to where Larowlan suggested because I couldn't make that work (although I'm not ruling out, of course, that there is a glaringly obvious solution that I missed but he didn't respond back, after all, it's weekend and there will be timezone differences involved).

After that much consideration and active involvement, I'm less inclined to say now that this is something core has to fix but a bit about this later. Modules have two (or three) avenues forward:

1. Use the full ritual from the drupal_goto() changelog. That allows the login to go through, it works as expected, only leaves an error message in the log. That shows that there is something amiss but the login process itself isn't harmed.

2. Stop using the hook_user_login() hook completely and move on to the submit handler where there is proper support for in-site redirect (setRedirectUrl()) or even external redirect (setResponse()).

3. Maybe Larowlan's middleware solution can work. I don't hold my breath now but there's the chance I mentioned above, this was my first foray into middleware territory, so I might have missed the solution. But it isn't that important any more, the previous two works. It'll only be important if some modul comes up with a scenario that can't be solved in the user login form submit handler. I think the usual redirect functioning can be solved there, so the need might not arise.

Now to the blame. What we can blame on Drupal 9.2 is not making the problem obvious earlier. The drupal_goto documentation is old, you can say that the writing was on the wall and module authors simply ignored to do the proper redirect routine. If they did, we would still get a log error now with 9.2 but that would be all, a message, and no actual problem with logging in.

So, long story short, I don't have the suggestion for a perfect solution. I'm more and more thinking that this is unlikely to change because, even if it came as a breaking change to some (probably many) users now, the redirect modules (or code) were clearly at fault, only that it didn't manifest itself. So, the best solution might be to push for a change in redirect module (or custom) code to use the proper approach.

For you and everybody else who caught the problem before they were locked out, this might be an easy one: wait for your current redirect module to be fixed (or, if it's your own code, fix it), then go on with 9.2. The problem is with those who already moved to 9.2 and are locked out now. It's not terribly complicated to solve if you know where to go and what to do but most Drupal admins are not programmers.