Redirect loop error when re-saving node with existing redirect

Right, but what exactly is the backtrace that that you see from that exception if you enable verbose logging?

It would also be useful to have steps to reproduce, I suspect it has something to do with renaming a node multiple times in a row and then back to the original name, resulting in a redirection loop.

Patch which removes the exception from being thrown when there is duplicate redirects, but instead logs the error instead and returns out.

Hi guys,
I stumbled across this issue as I have another separate problem caused by the same code. Basically, because that array isn't scoped to the current request, multiple passes trigger this exception incorrectly (e.g using subrquests with GraphQL).

I opened a separate issue in #3061173: scope foundRedirects to the current request from the request stack and I can see someone else stumbled into similar problems in #3059894: Redirect loop can occur in sub-requests

I guess what I'm saying is there seems to be multiple scenarios where this exception being triggered causes problems, so this patch would be definitely welcome.

Open to changing this to avoid an exception, but I think this might result in multiple logs when trying to access such a page?

Also, the expectations in the tests need to be updated.

Facing the same issue in my project. Any update or better approach suggested for this issue?

Created a patch without logging and compatible with D8.9

Attached is a re-rolled version of the patch where I have also moved the redirect prevent from a class member to an additional argument that gets passed through the recursive calls to findMatchingRedirect and findByRedirect, to avoid any parallelism issues.

I've also updated the automated test to reflect the difference in behaviour.

I'm not sure this is the right way to go about solving the original issue. Maybe there is a way to catch the RedirectLoopException before it breaks the node saving.

Also, would be good to add a test for the original issue.

#15 works for me, Thank you!
core v= 9.3.13
redirect v = 1.7

Rerolled; most of the changes were in the test file.

I couldn't apply the patch from #17, and since it isn't passing all the tests i'll try and work on it!

Looking through the changes made in patch #17 I noticed that the findMatchingRedirect() method now doesn't treat the redirect infinite loop. If we try to access a node that has a redirection lopp it'll break the website. This is why the functional tests were getting errors and the Kernel test was not getting the expected URL.

I'm sending a patch with some changes. I'm not very familiar with this module, but I suppose we could try to redirect the node with an infinite loop to a 404 page, and log the error. I did not change the tests, though, since I do not know if this is the correct aproach.

If there is anything that could be changed, feel free to give me any kind of feedback and i'll try to work on it. Also, if this solution is acceptable, I'm willing to work to adapt the tests.

I've adjusted patch from comment 20 since it was incorrectly using the create method and not using interfaces which caused a WSOD.

Re-rolled the patch from #21 due to the 1.9 release

attempt to fix Error: Class "Drupal\redirect\FormattableMarkup" not found in patch #22

fix logger

There are 20 open issues with the word "loop" in them, for example #3401916: Redirect loop wrong detection (service unavailable: RedirectLoopException), helping to identify duplicates, closing them and agreeing on the best solution between this and the other issue would be really helpful.

Re-rolling the patch to be used in composer.json patches for version 1.10. I'm not creating a MR since there seems to be a lack of clarity on the direction of the desired solution at this time.

Re-rolling again for 1.12

The patch in #29 causes the following error: TypeError: Drupal\redirect\RedirectRepository::__construct(): Argument #4 ($logger_factory) must be of type Drupal\Core\Logger\LoggerChannelFactoryInterface.
I created a new patch which resolves this errors.

Patch #30 applies properly to 8.x-1.12 and fixes the issue for me. Marking as RTBC

A patch can not be RTBC. The MR still needs work.

I merged 8.x-1.x into this branch and resolved conflicts. I also moved all newly-introduced arguments to the end of function signatures rather than adding them in the middle.

MR #92 does not work with version 1.13, which is now the latest release.

I am not having a problem applying what's in the merge request to 1.13 as patch (using the diff generated: https://git.drupalcode.org/project/redirect/-/merge_requests/92.diff).

Perhaps you have a conflict with another patch?

#30 patch works with version 1.12. Note that patch #30 does not work with version 1.13

MR #92 applies properly to version 1.13. Marking as RTBC