opcodesio / log-viewer

Fast and beautiful Log Viewer for Laravel
https://log-viewer.opcodes.io
MIT License
3.39k stars 237 forks source link

Fixed issue with search API optional parameters #308

Closed artshade closed 6 months ago

artshade commented 6 months ago

Dear Developers,

Thank you for the project! :rocket:

Please consider checking out this light fix regarding the issue which results in the following exception if no file nor any severity is selected:

Cannot assign string to property Opcodes\LogViewer\Readers\MultipleLogReader::$exceptLevels of type ?array {"exception":"[object] (TypeError(code: 0): Cannot assign string to property Opcodes\\LogViewer\\Readers\\MultipleLogReader::$exceptLevels of type ?array at /var/www/html/vendor/opcodesio/log-viewer/src/Readers/MultipleLogReader.php:39)
[stacktrace]
#0 /var/www/html/vendor/opcodesio/log-viewer/src/Http/Controllers/LogsController.php(66): Opcodes\\LogViewer\\Readers\\MultipleLogReader->exceptLevels('')
#1 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/ControllerDispatcher.php(46): Opcodes\\LogViewer\\Http\\Controllers\\LogsController->index(Object(Illuminate\\Http\\Request))
#2 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/Route.php(260): Illuminate\\Routing\\ControllerDispatcher->dispatch(Object(Illuminate\\Routing\\Route), Object(Opcodes\\LogViewer\\Http\\Controllers\\LogsController), 'index')

Possibly, the issue is related to #273.

Best and kind regards :sparkles:

arukompas commented 6 months ago

hey @artshade , thanks for the PR!

I took it for a spin and realised why this issue was happening for some but not all users.

Most Laravel setups use the \Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull middleware for all routes (defined in the HTTP kernel), and so the Log Viewer's $request->query('exclude_levels', []) would default to the empty array instead of an empty string.

But, when the middleware above is not used, Laravel passes the empty string as it is (it's not null, therefore does not default to the empty array).

I found that the only fix needed is the one you have done in the frontend logViewer store! 😄 The change makes sure not to submit the parameter at all if the array is empty, which is what we want. Then the controller can safely default to the empty array.

If you could revert the changes done on the LogsController, then I could merge the PR and tag a release.

artshade commented 6 months ago

hey @artshade , thanks for the PR!

I took it for a spin and realised why this issue was happening for some but not all users.

Most Laravel setups use the \Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull middleware for all routes (defined in the HTTP kernel), and so the Log Viewer's $request->query('exclude_levels', []) would default to the empty array instead of an empty string.

But, when the middleware above is not used, Laravel passes the empty string as it is (it's not null, therefore does not default to the empty array).

Dear @arukompas ,

Thank you very much for the reply!

Exactly this it seems! The local project does have Laravel 10.38 active, and the mentioned class ConvertEmptyStringsToNull exists and is even added into autoload classmaps but is not used in any actual functional code.

Considering the official defaults of the current Laravel project, the middleware is implemented but not used, too.

Applied the requested changes. Are you sure these are the only ones you want in this PR?

Best and kind regards! :sparkles:

arukompas commented 6 months ago

Thanks a lot @artshade , tagged a new release v3.1.11 💪

Happy holidays! 🎉