Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Xdebug has detected a possible infinite loop #448

Open
ajithMagic opened this issue Aug 21, 2024 · 6 comments
Open

Xdebug has detected a possible infinite loop #448

ajithMagic opened this issue Aug 21, 2024 · 6 comments

Comments

@ajithMagic
Copy link

Hi,
I'm using Saloon v3.10.0 and saloon/laravel-plugin v3.5.0 running on a Laravel project v10.48.19. Was using this package for very long time and I never had an issue like this before.

Today, while I was trying to generate a connector/request I got the following error.
Xdebug has detected a possible infinite loop, and aborted your script with a stack depth of '512' frames.

@ajithMagic
Copy link
Author

I have googled and found there was an issue with the Laravel framework itself on the above issue laravel/framework#49968. But it was fixed on the next release.

So I have gone through the saloon/laravel-plugin package to understand how this works.
This is the function where infinite loop error hits. https://github.com/laravel/framework/blob/05a9554ac0c7361504a54e350787aba84d028ce7/src/Illuminate/Console/GeneratorCommand.php#L204

specifically this line. https://github.com/laravel/framework/blob/05a9554ac0c7361504a54e350787aba84d028ce7/src/Illuminate/Console/GeneratorCommand.php#L216

Since that is at the laravel level, I looked into the plugin itself and found the function,https://github.com/saloonphp/laravel-plugin/blob/15be0d587e61f11076f93d6df30561b88ff837c1/src/Console/Commands/MakeCommand.php#L58 is causing the issue.

Removing the $rootNamespace from this line https://github.com/saloonphp/laravel-plugin/blob/15be0d587e61f11076f93d6df30561b88ff837c1/src/Console/Commands/MakeCommand.php#L62 has fixed the issue.

@ajithMagic
Copy link
Author

I'm happy to make a PR for the above but I'm not certain that is the right function and correct refactoring to fix the issue. I'm happy to provide further details if required and assist on this.

@juse-less
Copy link
Contributor

@ajithMagic Hey.

It'd be good to know why Laravel can't find the class.
They call the qualifyClass() method that you link to in your 2nd link.
If it can't resolve the class name, it'll call itself again, passing the result of the getDefaultNamespace().
Like your 4th link shows, Saloon is overwriting that method.
Whatever Saloon returns is then not found by qualifyClass(), which is then causing qualifyClass() to again call itself with the result of getDefaultNamespace(), that, again, Saloon has overwritten.
Rinse and repeat.

Even if removing the $rootNamespace variable seemingly solves it, it'd be good to know why.
Like.. what's Saloon actually returning?
Is it possible that some backslash replacement and/or concatenation goes wrong?
Is it possible Saloon need to handle values from the config file differently?
Like.. ensuring the namespace value starts and/or ends with backslash.

It's a bit inconvenient for me to test right now, as I'm on my phone.
But I hope it was somewhat helpful. 🙂

@ajithMagic
Copy link
Author

Hi @juse-less based,
based on your comment I have looked into the code again specifically MakeCommand class. Found the function is not stripping the App from the string.

protected function getNamespaceFromIntegrationsPath(): string
    {
        $namespace = (array)str_replace(['\\App', '\\app'], '', str_replace('/', '\\', str_replace(base_path(), '', config('saloon.integrations_path'))));

        return $namespace[0];
    }

Specifically str_replace(['App', 'app'], '', ...) this piece of code. Because the rest of the str_replace function returns the string "App\Http\Integrations" which doesn't have \ in front of it.

@juse-less
Copy link
Contributor

@ajithMagic Hmm. I wonder if it'd be better to replace that specific str_replace() with a preg_replace() instead.
Maybe something like

$namespace = preg_replace('/^\\?app/i', '', ...);

return $namespace;

Just be aware that this is a breaking change, as we're matching from the start, rather than any part of the namespace.
I haven't tested the regexp, so it's possible there's a typo.
It'd be good with a second opinion about a possible solution, though.

@ajithMagic
Copy link
Author

Agreed 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants