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.

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

Add support for ServiceException handlers #129

Merged
merged 4 commits into from
Feb 17, 2021
Merged

Add support for ServiceException handlers #129

merged 4 commits into from
Feb 17, 2021

Conversation

shalvah
Copy link
Contributor

@shalvah shalvah commented Feb 16, 2021 •
edited
Loading

Status

READY

Description

Allow the user to specify what the library should do when it encounters an error while sending data to Honeybadger. For instance:

$honeybadger = Honeybadger\Honeybadger::new([
    'api_key' => 'abc123',
    'service_exception_handler' => function (ServiceException $e) {
        $logger->warning($e);
    },
]);

For framework integrations (Laravel and Lumen), the user won't have to worry about this; we'll automatically set the handler to log the exception.

Note: the current default behaviour is to throw, so we need to either change it or prominently mention the behaviour in our docs.

Fixes #121.

Related PRs

Laravel - honeybadger-io/honeybadger-laravel#70
Docs - https://github.com/honeybadger-io/docs/pull/90

@shalvah shalvah mentioned this pull request Feb 16, 2021
Merged
@shalvah shalvah requested a review from sixlive February 16, 2021 21:17
sixlive
sixlive approved these changes Feb 17, 2021
View reviewed changes
@joshuap
Copy link
Member

joshuap commented Feb 17, 2021

@shalvah @sixlive would it be a good idea to log by default for everyone? I'm trying to think of a situation where someone would want a service exception thrown—the only one I've come up with is if I'm building a 3rd-party integration on top of honeybadger-php, which is a minority of users.

@sixlive
Copy link
Collaborator

sixlive commented Feb 17, 2021

@joshuap I think the problem is we won't know what type of logging they are using. I can't think of anything we could do here that isn't too opinionated with a smart default.... unless I'm just not thinking of something obvious, which could totally be the case.

@shalvah
Copy link
Contributor Author

shalvah commented Feb 17, 2021 •
edited
Loading

Yeah @joshuap, I'd favour logging too, but because we don't know what kind of logging they're doing, or where they're logging to, our options are limited.

We could write to a file, but what if they ordinarily log to stdout, or keep log files in a different location?

Still, it's something we can consider further and change later on.

@shalvah shalvah merged commit 95ea3d9 into master Feb 17, 2021
@shalvah shalvah deleted the serviceexception-handler branch February 17, 2021 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sixlive sixlive sixlive approved these changes

Assignees

@shalvah shalvah

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feedback regarding service errors
3 participants
@shalvah @joshuap @sixlive