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.

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

Add on_breadcrumb callbacks to replace before_breadcrumb_callbacks #686

Merged
merged 6 commits into from
Aug 23, 2021

Conversation

imjoehaines
Copy link
Contributor

@imjoehaines imjoehaines commented Aug 23, 2021

Goal

Adds on_breadcrumb callbacks, which will replace before_breadcrumb_callbacks in the next major release

The API is similar to on_error callbacks:

callback = proc do |breadcrumb|
  breadcrumb.metadata = { hello: 'world' }
end

Bugsnag.configure do |config|
  config.add_on_breadcrumb(callback)
  config.remove_on_breadcrumb(callback)
end

Bugsnag.add_on_breadcrumb(callback)
Bugsnag.remove_on_breadcrumb(callback)

The semantics also follow on_error callbacks:

  • returning false from a callback will discard the breadcrumb and stop any subsequent callbacks from running
  • any errors raised by a callback will be rescued and logged. Any subsequent callbacks will still be run and the breadcrumb will not be discarded
  • any object responding to #call is a valid callback

Changeset

  • add Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList which manages the callbacks and can call them with the correct semantics
  • add Bugnsag#add_on_breadcrumb, Bugnsag#remove_on_breadcrumb, Bugnsag#add_on_breadcrumb and Bugnsag::Configuration#remove_on_breadcrumb
  • modified Bugsnag#leave_breadcrumb to call the new callbacks via the OnBreadcrumbCallbackList

Testing

This is tested both in tests specifically for OnBreadcrumbCallbackList to ensure the semantics are correct and as part of the main set of Bugsnag tests to ensure they are called when leaving a breadcrumb

There are big whitespace changes in bugsnag_spec.rb to split before_breadcrumb_callbacks into their own describe, so it's easier to review that file by ignoring whitespace

@imjoehaines imjoehaines merged commit 36a1c7b into next Aug 23, 2021
@imjoehaines imjoehaines deleted the on-breadcrumb-callbacks branch August 23, 2021 16:08
@imjoehaines imjoehaines mentioned this pull request Sep 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants