nWidart / laravel-modules

Module Management In Laravel
https://docs.laravelmodules.com
MIT License
5.49k stars 952 forks source link

ExceptionHandler::report() mu st be an instance of Exception #574

Closed Alercard closed 4 years ago

Alercard commented 6 years ago

I'm working with seeds, but I got this error message:

"Symfony\Component\Debug\Exception\FatalThrowableError : Type error: Argument 1 passed to NunoMaduro\Collision\Adapters\Laravel\ExceptionHandler::report() mu st be an instance of Exception, instance of Error given, called in C:\proyectos\ laravel\onyx\vendor\nwidart\laravel-modules\src\Commands\SeedCommand.php on line 179"

I don't understand this issue, because I can't read the real error message and I don't know what happens!

I'm using Laravel 5.6 and nwidart/laravel-modules v3.0

It falls in this instruction: $profile->options()->sync($options);

Thanks for your help!

nWidart commented 6 years ago

Hi,

What version of php are you using?

Ps: There is no need to post this in collision.

Alercard commented 6 years ago

My php version is 7.2.5

I found in SeedCommand.php file, on handle function, in catch section, param $e is an Error class, not an Exception class and that trigger a new exception who shows me another error message.

That Error class, shows me the true error. I hope this can helps to solve this issue!

Alercard commented 6 years ago

By the way, thanks for this plugin!

nWidart commented 6 years ago

Indeed, but they should be an implementation of Throwable which is what is typehinted.

I can't reproduce your issue on my setup, php 7.1 or 7.2

Alercard commented 6 years ago

Ok, I'll explain what happens.

  1. I created two models for a ManyToMany relationship. In this case, Profile and Option Models. I used "php artisan module:make-model ..." instruction

  2. In Profile.php, I reference to Option Model by "use Option;" instruction and I wrote the function: public function options() { return $this->belongsToMany(Option::class, 'sec_permissions', 'profile_id','option_id')->withTimestamps(); }

  3. When I use "$profile->options()->sync($options);" intruction in the seeder php file, that generate the Error class. In this case, the error class said: "Class Option not founded", so I replaced the instruction "use Option;" for "use Modules\Security\Entities\Option;" and this solve my true issue.

I hope with theese instructions helps to reproduce this issue.

ebisbal commented 6 years ago

The solution i proposed in issue #579 is that you must change functions renderException and reportException. In that functions, they receive a Throwable object $e which is passed to functions renderForConsole and report but this functions need to receive an Exception object. Then you must cast the Throwable object to an Exception object.

garbinmarcelo commented 6 years ago

Same problem here, did you correct?

ghost commented 6 years ago

Same problem. @nWidart Maybe i can make an fix PR?

nWidart commented 6 years ago

@aios of course yes. 👍 Keep in mind the solution is not what's done in this PR ( https://github.com/nWidart/laravel-modules/pull/598)

ghost commented 6 years ago

@nWidart i looked at here your answer - fine - i must work with it.

sthaiya commented 5 years ago

I can confirm this is still an issue with latest version (v4.0.0) on laravel 5.7 and PHP 7.1.26

mrcage commented 5 years ago

Same for me, issue still exists. Laravel 5.7, laravel-modules 4, PHP 7.1.11

This is my (very simple) seeder:

<?php

namespace Modules\Logistics\Database\Seeders;

use Illuminate\Database\Seeder;
use Illuminate\Database\Eloquent\Model;

class SuppliersSeederTableSeeder extends Seeder
{
    /**
     * Run the database seeds.
     *
     * @return void
     */
    public function run()
    {
        Model::unguard();

        Modules\Logistics\Entities\Supplier::create([
            'name' => str_random(10),
            'address' => str_random(10),
            'category' => str_random(10),            
        ]);
    }
}
antick commented 5 years ago

@nWidart i looked at here your answer - fine - i must work with it.

Hey man, are you still working on a PR for this?

Do you see a solution other than #598? I tried but couldn't find any other solution than changing the Throwable to Exception in the handle method and reportException parameter.

nWidart commented 5 years ago

The ultimate solution is to not have an error in your migration 😄

antick commented 5 years ago

The ultimate solution is to not have an error in your migration

It's like suggesting somebody to not drive a car to avoid accidents. :laughing:

Just kidding man.

I love this package and hopefully, this gets fixed because a lot of first-time users might be scratching their heads to figure this out.

Although it is always a silly mistake that lands everyone to this error.

mrcage commented 5 years ago

Agree with @pankajsanam ; of course its goood to have no errors in migrations, but if there is an issue, its soo much easier to debug if there is the appropriate error message. With the current status, not even anayzing the stack trace in the laravel.log helps nail down the issue.

nWidart commented 5 years ago

Don't get me wrong I'd like to have this fixed, but currently, the proposed solutions don't fully solve the issue.

stephandesouza commented 5 years ago

@nWidart may i help you with this

stephandesouza commented 5 years ago

Reply from #744, but what are you expecting than?

Laravel renderForConsole() and report() only accepts Exception which implements Throwable. But not all Throwable instance are in fact Exception because internal PHP errors throws Error (example used).

nWidart commented 5 years ago

Well if I knew the issue wouldn't exist anymore.

I know that there was first Exception , then Throwable, then back, etc. never fully fixing the issue.

stephandesouza commented 5 years ago

Remove the try/catch exception from handle() method and leave to Framework to manager this, is not a solution?

web2sign commented 5 years ago

I had this problem... I dont get this error on other module except for Ads module O_O

nWidart commented 5 years ago

It means you have some error (like a syntax error) in a migration of this module.

web2sign commented 5 years ago

It means you have some error (like a syntax error) in a migration of this module.

Yeah I've found the culprit earlier it's a misspelled Carbon ugh

use Carbom\Carbon;

badrul1329 commented 5 years ago

@nWidart is this really solved? i'm getting the error in Laravel Modules v5.0, i am using laravel v5.8.18. If Exception instead of Throwable etc not the solution, then What's the permanent solution of this error?

nWidart commented 5 years ago

It's not solved, and don't have a solution yet, hence the open status of this ticket.

emmanuelleabendan commented 5 years ago

I just had this same error

Symfony\Component\Debug\Exception\FatalThrowableError : Argument 1 passed to NunoMaduro\Collision\Adapters\Laravel\ExceptionHandler::report() must be an instance of Exc eption, instance of Error given, called in .....

All I did is reference the Model at the top

Example: use Modules\Admin\Entities\Modelname;

And everything works.

nWidart commented 4 years ago

I'll close this as there hasn't been a reply in a while, feel free to comment if needed.

mnestorov commented 4 years ago

I just had this same error

Symfony\Component\Debug\Exception\FatalThrowableError : Argument 1 passed to NunoMaduro\Collision\Adapters\Laravel\ExceptionHandler::report() must be an instance of Exc eption, instance of Error given, called in .....

All I did is reference the Model at the top

Example: use Modules\Admin\Entities\Modelname;

And everything works.

I can confirm that.