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

Dont register redis watcher if the redis service is not bound into the container. #1259

Merged
merged 1 commit into from
Oct 6, 2022

Conversation

mad-briller
Copy link
Contributor

@mad-briller mad-briller commented Oct 6, 2022

When installing Telescope into a Laravel application which has opted out of Redis support by removing the RedisServiceProvider in app.php, a BindingResolutionException is thrown during auto-discovery:

 ⇒  composer require --dev laravel/telescope
Info from https://repo.packagist.org: #StandWithUkraine
Using version ^4.9 for laravel/telescope
./composer.json has been updated
Running composer update laravel/telescope
Loading composer repositories with package information
Updating dependencies
Lock file operations: 1 install, 0 updates, 0 removals
  - Locking laravel/telescope (v4.9.4)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 1 install, 0 updates, 0 removals
  - Downloading laravel/telescope (v4.9.4)
  - Installing laravel/telescope (v4.9.4): Extracting archive
Generating optimized autoload files
> Illuminate\Foundation\ComposerScripts::postAutoloadDump
> @php artisan package:discover --ansi

   Illuminate\Contracts\Container\BindingResolutionException

  Target class [redis] does not exist.

  at vendor/laravel/framework/src/Illuminate/Container/Container.php:877
    873▕
    874▕         try {
    875▕             $reflector = new ReflectionClass($concrete);
    876▕         } catch (ReflectionException $e) {
  ➜ 877▕             throw new BindingResolutionException("Target class [$concrete] does not exist.", 0, $e);
    878▕         }
    879▕
    880▕         // If the type is not instantiable, the developer is attempting to resolve
    881▕         // an abstract type such as an Interface or Abstract Class and there is

  1   [internal]:0
      Illuminate\Foundation\Application::Illuminate\Foundation\{closure}(Object(Laravel\Telescope\TelescopeServiceProvider))

      +18 vendor frames
  20  [internal]:0
      Illuminate\Foundation\Application::Illuminate\Foundation\{closure}(Object(Laravel\Telescope\TelescopeServiceProvider))
Script @php artisan package:discover --ansi handling the post-autoload-dump event returned with error code 1

This can be prevented by disabling the watcher using a .env value, or modifying the telescope.php config file, but a user who is installing Telescope for the first time wont expect to have to do such a thing to get it working.

This pr makes the RedisWatcher register itself only when redis is bound into the application container to avoid this issue.

Thanks for your time.

@taylorotwell taylorotwell merged commit a316d6d into laravel:4.x Oct 6, 2022
@mad-briller mad-briller deleted the unbound-redis branch October 6, 2022 14:05
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

Successfully merging this pull request may close these issues.

2 participants