8000 [8.x] Change exception's handler ignore method to public by smujaddid · Pull Request #36548 · laravel/framework · GitHub
[go: up one dir, main page]
Skip to content

[8.x] Change exception's handler ignore method to public #36548

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

[8.x] Change exception's handler ignore method to public #36548

wants to merge 5 commits into from

Conversation

smujaddid
Copy link
Contributor

Making this ignore method as public can helps when developing an app with sub-modules (package) as the main app will not always know what exception should be ignored from sub-modules.

For example, in a sub-module service provider:

// Assume this will be resolved as App\Exceptions\Handler 
// which extends Illuminate\Foundation\Exceptions\Handler
$handler = $this->app->make(\Illuminate\Contracts\Debug\ExceptionHandler::class);
$handler->ignore(\Laravel\Passport\Exceptions\OAuthServerException::class);

This way will allow sub-modules add any exception (not only its own exception, but also any other exception) that considered should be ignored for reporting. If the sub-module is not used/included in main app, then the exception will not be added to dontReport list.

Another example of handy public method in Laravel's exception handler for a sub-module is reportable and renderable

// Assume this will be resolved as App\Exceptions\Handler
// which extends Illuminate\Foundation\Exceptions\Handler
$handler = $this->app->make(\Illuminate\Contracts\Debug\ExceptionHandler::class);
$handler->reportable(function (\My\Module\Exception\InvalidUsernameException $e) {
    ...
});

Rather than creating this in App\Exceptions\Handler class:

public function dontReport(string $class)
{
    $this->ignore($class);

    return $this;
}

It is better to call ignore directly than dontReport wrapper method.

@GrahamCampbell
Copy link
Member

This is a breaking change, and needs to go to master, if at all. You can already make this method public in your app's own exception handler by overriding the method and increasing its visibility. This is exactly why the change is breaking though, since protected methods can be overwritten to be public, but public methods cannot be overwritten to be protected.

@smujaddid
Copy link
Contributor Author

This is a breaking change, and needs to go to master, if at all. You can already make this method public in your app's own exception handler by overriding the method and increasing its visibility. This is exactly why the change is breaking though, since protected methods can be overwritten to be public, but public methods cannot be overwritten to be protected.

I will change the target branch to master instead

@smujaddid smujaddid changed the base branch from 8.x to master March 11, 2021 01:19
@smujaddid smujaddid changed the title [8.x] Change exception's handler ignore method to public [9.x] Change exception's handler ignore method to public Mar 11, 2021
@smujaddid smujaddid closed this Mar 11, 2021
@smujaddid smujaddid changed the title [9.x] Change exception's handler ignore method to public [8.x] Change exception's handler ignore method to public Mar 11, 2021
@smujaddid smujaddid mentioned this pull request Mar 11, 2021
Merged
@smujaddid smujaddid deleted the public-ignore-method branch March 11, 2021 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@smujaddid @GrahamCampbell @TBlindaruk @tomirons
0