amir9480 / vscode-laravel-extra-intellisense

This extension adds extra autocompletion for laravel projects to VSCode.
https://marketplace.visualstudio.com/items?itemName=amiralizadeh9480.laravel-extra-intellisense
MIT License
3.44k stars 54 forks source link

Plugin error is logged in laravel's log #31

Closed ashrafkamarudin closed 3 years ago

ashrafkamarudin commented 4 years ago

I found 3 issue within the plugin all error is logged in laravel's log

The first issue is :

[2020-09-25 19:51:40] local.ERROR: Uncaught Error: Class 'Arr' not found in Command line code:1
Stack trace:
#0 {main}
  thrown {"exception":"[object] (Symfony\\Component\\Debug\\Exception\\FatalErrorException(code: 1): Uncaught Error: Class 'Arr' not found in Command line code:1
Stack trace:
#0 {main}
  thrown at Command line code:1)
[stacktrace]
#0 {main}
"} 

I dig down and found the caused. It was from file /src/authprovider.ts on line 55. The caused of it was the usage of Arr::flatten(). This will cause error for users who don't have Illuminate\support\Arr registered as alias.

Second Issue I found is:

After fixing the Arr::flatten() issue, I got a new error.

[2020-10-06 06:15:03] local.ERROR: Uncaught ReflectionException: Class App\Policies\ModelPolicy does not exist in Command line code:1
Stack trace:
#0 Command line code(1): ReflectionClass->__construct('App\\Policies\\Mo...')
#1 [internal function]: {closure}('App\\Policies\\Mo...', 'App\\Model')
#2 Command line code(1): array_map(Object(Closure), Array, Array)
#3 {main}
  thrown {"exception":"[object] (Symfony\\Component\\Debug\\Exception\\FatalErrorException(code: 1): Uncaught ReflectionException: Class App\\Policies\\ModelPolicy does not exist in Command line code:1
Stack trace:
#0 Command line code(1): ReflectionClass->__construct('App\\\\Policies\\\\Mo...')
#1 [internal function]: {closure}('App\\\\Policies\\\\Mo...', 'App\\\\Model')
#2 Command line code(1): array_map(Object(Closure), Array, Array)
#3 {main}
  thrown at Command line code:1)
[stacktrace]
#0 {main}
"}

I see that you added a new feature, autocompleting policies. But for people who don't use policies and actually have a default policy registered when you first installed laravel at \app\AuthServiceProvider.php will receive this error.

AuthServiceProvider.php

/**
 * The policy mappings for the application.
 *
 * @var array
 */
 protected $policies = [
    'App\Model' => 'App\Policies\ModelPolicy',
 ];

for user who don't use policy and didn't have the file in that location, that error log will consistently came out. it's best if you have some kind of handler for this. I find the fix is pretty simple. Just comment the line and it works fine. But I think this should be handled within the plugin itself.

The third issue is :

[2020-10-06 06:31:40] local.ERROR: Uncaught Error: Call to undefined method App\Http\Kernel::getRouteMiddleware() in Command line code:1
Stack trace:
#0 {main}
  thrown {"exception":"[object] (Symfony\\Component\\Debug\\Exception\\FatalErrorException(code: 1): Uncaught Error: Call to undefined method App\\Http\\Kernel::getRouteMiddleware() in Command line code:1
Stack trace:
#0 {main}
  thrown at Command line code:1)
[stacktrace]
#0 {main}
"} 

I don't know much about this one since i didn't look into it. But it is an issue nonetheless.

amir9480 commented 4 years ago

Hi @ashrafkamarudin

First issue: It's weird because Arr class is an alias class in laravel. https://github.com/laravel/laravel/blob/c66546e75fcbf208d2884b5ac7a3a858137753a3/config/app.php#L194

Second issue: About AuthServiceProvider, it's a commented line by default and should not be filled with non-existing class. https://github.com/laravel/laravel/blob/c66546e75fcbf208d2884b5ac7a3a858137753a3/app/Providers/AuthServiceProvider.php#L16

Third issue: It's weird, I used this method to get middleware names. You can see it's definition here:

https://github.com/laravel/framework/blob/5b756ae90a16ca224ad5283956eca6d3e15aa278/src/Illuminate/Foundation/Http/Kernel.php#L434

Make sure you are using the latest version of laravel in your project.

ashrafkamarudin commented 4 years ago

I see. yes i've check the files it seems that these changes is applied to Laravel's 5.8 and above. So the one who used Laravel 5.7 and below will get these errors in their laravel log.

Do you plan on supporting old laravel ?

also btw, I think you should update your log handler seems to not be working as it logs to laravel's log file on this line https://github.com/amir9480/vscode-laravel-extra-intellisense/blob/740dd4f8a82d23d55ea003024f9aedfd0c2a0d8b/src/helpers.ts#L66

maybe you can try it with

$this->app['log']->getLogger()->pushHandler(new \\Monolog\\Handler\\NullHandler());
amir9480 commented 4 years ago

@ashrafkamarudin

No, I do not have any plans to support older versions of laravel. I highly recommend you to update your laravel application to the latest version.

Also about this handler please make a pull request to fix, So I can check and use your solution, thanks.

ashrafkamarudin commented 4 years ago

Alright, I'll do it.

mwkcoding commented 3 years ago

@amir9480 You should consider using FQN instead of assuming people have their aliases registerered. Some people might upgrade their Laravel projects without updating configs cause it's never included in the upgrade guides unless it's a breaking change. Aliases will NEVER be a breaking change in configs. They're made for the convenience of the developer. They're optional, not required.

amir9480 commented 3 years ago

@mwkcoding Hi I don't understand what you mean.

Can you please send me a sample code to disable the Laravel log? The current code is: https://github.com/amir9480/vscode-laravel-extra-intellisense/blob/master/src/helpers.ts#L80

mwkcoding commented 3 years ago

@amir9480 I mean for example with helper classes like Arr https://github.com/amir9480/vscode-laravel-extra-intellisense/blob/master/src/AuthProvider.ts#L55 Instead of expecting there to be an alias for it in the project, use the FQN Illuminate\Support\Arr. That would solve this error on Laravel ^6.0 which might not have the class registered as an alias in their config from upgrading from Laravel 5.

amir9480 commented 3 years ago

@mwkcoding Got it.

Contributions are welcome. Please fix this and make a pull request.