diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ce18a806..a78b81907 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,57 @@ Changelog ========= +## v6.23.0 (21 September 2021) + +### Enhancements + +* Sessions will now be delivered every 10 seconds, instead of every 30 seconds + | [#680](https://github.com/bugsnag/bugsnag-ruby/pull/680) +* Log errors that prevent delivery at `ERROR` level + | [#681](https://github.com/bugsnag/bugsnag-ruby/pull/681) +* Add `on_breadcrumb` callbacks to replace `before_breadcrumb_callbacks` + | [#686](https://github.com/bugsnag/bugsnag-ruby/pull/686) +* Add `context` attribute to configuration, which will be used as the default context for events. Using this option will disable automatic context setting + | [#687](https://github.com/bugsnag/bugsnag-ruby/pull/687) + | [#688](https://github.com/bugsnag/bugsnag-ruby/pull/688) +* Add `Bugsnag#breadcrumbs` getter to fetch the current list of breadcrumbs + | [#689](https://github.com/bugsnag/bugsnag-ruby/pull/689) +* Add `time` (an ISO8601 string in UTC) to `device` metadata + | [#690](https://github.com/bugsnag/bugsnag-ruby/pull/690) +* Add `errors` to `Report`/`Event` containing an array of `Error` objects. The `Error` object contains `error_class`, `error_message`, `stacktrace` and `type` (always "ruby") + | [#691](https://github.com/bugsnag/bugsnag-ruby/pull/691) +* Add `original_error` to `Report`/`Event` containing the original Exception instance + | [#692](https://github.com/bugsnag/bugsnag-ruby/pull/692) +* Add `request` to `Report`/`Event` containing HTTP request metadata + | [#693](https://github.com/bugsnag/bugsnag-ruby/pull/693) +* Add `add_metadata` and `clear_metadata` to `Report`/`Event` + | [#694](https://github.com/bugsnag/bugsnag-ruby/pull/694) +* Add `set_user` to `Report`/`Event` + | [#695](https://github.com/bugsnag/bugsnag-ruby/pull/695) + +### Fixes + +* Avoid starting session delivery thread when the current release stage is not enabled + | [#677](https://github.com/bugsnag/bugsnag-ruby/pull/677) + +### Deprecated + +* `before_breadcrumb_callbacks` have been deprecated in favour of `on_breadcrumb` callbacks and will be removed in the next major release +* For consistency with Bugsnag notifiers for other languages, a number of methods have been deprecated in this release. The old options will be removed in the next major version | [#676](https://github.com/bugsnag/bugsnag-ruby/pull/676) + * The `notify_release_stages` configuration option has been deprecated in favour of `enabled_release_stages` + * The `auto_capture_sessions` and `track_sessions` configuration options have been deprecated in favour of `auto_track_sessions` + * The `enabled_automatic_breadcrumb_types` configuration option has been deprecated in favour of `enabled_breadcrumb_types` + * The `Report` class has been deprecated in favour of the `Event` class + * The `Report#meta_data` attribute has been deprecated in favour of `Event#metadata` + * The `Breadcrumb#meta_data` attribute has been deprecated in favour of `Breadcrumb#metadata` + * The `Breadcrumb#name` attribute has been deprecated in favour of `Breadcrumb#message` + * The breadcrumb type constants in the `Bugsnag::Breadcrumbs` module has been deprecated in favour of the constants available in the `Bugsnag::BreadcrumbType` module + For example, `Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE` is now available as `Bugsnag::BreadcrumbType::ERROR` +* `Report#exceptions` has been deprecated in favour of the new `errors` property +* `Report#raw_exceptions` has been deprecated in favour of the new `original_error` property +* Accessing request data via `Report#metadata` has been deprecated in favour of using the new `request` property. Request data will be moved out of metadata in the next major version +* The `Report#add_tab` and `Report#remove_tab` methods have been deprecated in favour of the new `add_metadata` and `clear_metadata` methods + ## v6.22.1 (11 August 2021) ### Fixes diff --git a/TESTING.md b/TESTING.md index 28afd94ff..e10187cc3 100644 --- a/TESTING.md +++ b/TESTING.md @@ -79,3 +79,24 @@ RUBY_TEST_VERSION=2.6 RAILS_VERSION=6 docker-compose run --use-aliases ruby-maze ``` In order to avoid running flakey or unfinished tests, the tag `"not @wip"` can be added to the tags option. This is recommended for all CI runs. If a tag is already specified, this should be added using the `and` keyword, e.g. `--tags "@rails6 and not @wip"` + +## Manually testing queue libraries + +To help manually test queue libraries and Active Job with various queue adapters, you can use [the `run-ruby-integrations` script](./features/fixtures/run-ruby-integrations). This takes care of installing your local copy of Bugsnag, booting Rails, setting up the database and booting the queue library + +As with the end-to-end tests, only Bugsnag employees can run this script as it relies on the same private resources + +The script will default to booting Sidekiq: + +``` +# run the rails_integrations fixture with sidekiq +$ ./features/fixtures/run-rails-integrations +``` + +The script can also run Resque, Que or Delayed Job: + +``` +$ ./features/fixtures/run-rails-integrations resque +$ ./features/fixtures/run-rails-integrations que +$ ./features/fixtures/run-rails-integrations delayed_job +``` diff --git a/VERSION b/VERSION index 261151bf6..0b31cc63b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -6.22.1 +6.23.0 diff --git a/features/fixtures/docker-compose.yml b/features/fixtures/docker-compose.yml index 953329ba2..ccb7f6fb0 100644 --- a/features/fixtures/docker-compose.yml +++ b/features/fixtures/docker-compose.yml @@ -129,6 +129,7 @@ services: - SQL_ONLY_BREADCRUMBS - ADD_ON_ERROR - USE_DEFAULT_AUTO_CAPTURE_SESSIONS + - ADD_REQUEST_ON_ERROR restart: "no" rails4: @@ -165,6 +166,7 @@ services: - SQL_ONLY_BREADCRUMBS - ADD_ON_ERROR - USE_DEFAULT_AUTO_CAPTURE_SESSIONS + - ADD_REQUEST_ON_ERROR restart: "no" rails5: @@ -201,6 +203,7 @@ services: - SQL_ONLY_BREADCRUMBS - ADD_ON_ERROR - USE_DEFAULT_AUTO_CAPTURE_SESSIONS + - ADD_REQUEST_ON_ERROR restart: "no" rails6: @@ -237,6 +240,7 @@ services: - SQL_ONLY_BREADCRUMBS - ADD_ON_ERROR - USE_DEFAULT_AUTO_CAPTURE_SESSIONS + - ADD_REQUEST_ON_ERROR restart: "no" networks: default: @@ -261,6 +265,8 @@ services: - RUN_AT_EXIT_HOOKS - ACTIVE_JOB_QUEUE_ADAPTER restart: "no" + ports: + - "3000:3000" sidekiq: build: diff --git a/features/fixtures/plain/app/app.rb b/features/fixtures/plain/app/app.rb index 37524117e..a0c3b3383 100644 --- a/features/fixtures/plain/app/app.rb +++ b/features/fixtures/plain/app/app.rb @@ -16,7 +16,7 @@ def configure_using_environment conf.auto_notify = ENV["BUGSNAG_AUTO_NOTIFY"] != "false" conf.ignore_classes << lambda { |ex| ex.class.to_s == ENV["BUGSNAG_IGNORE_CLASS"] } if ENV.include? "BUGSNAG_IGNORE_CLASS" conf.meta_data_filters << ENV["BUGSNAG_META_DATA_FILTERS"] if ENV.include? "BUGSNAG_META_DATA_FILTERS" - conf.notify_release_stages = [ENV["BUGSNAG_NOTIFY_RELEASE_STAGE"]] if ENV.include? "BUGSNAG_NOTIFY_RELEASE_STAGE" + conf.enabled_release_stages = [ENV["BUGSNAG_NOTIFY_RELEASE_STAGE"]] if ENV.include? "BUGSNAG_NOTIFY_RELEASE_STAGE" conf.project_root = ENV["BUGSNAG_PROJECT_ROOT"] if ENV.include? "BUGSNAG_PROJECT_ROOT" conf.proxy_host = ENV["BUGSNAG_PROXY_HOST"] if ENV.include? "BUGSNAG_PROXY_HOST" conf.proxy_password = ENV["BUGSNAG_PROXY_PASSWORD"] if ENV.include? "BUGSNAG_PROXY_PASSWORD" diff --git a/features/fixtures/rails3/app/config/initializers/bugsnag.rb b/features/fixtures/rails3/app/config/initializers/bugsnag.rb index baa9c5d3f..ca5b93ca7 100644 --- a/features/fixtures/rails3/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails3/app/config/initializers/bugsnag.rb @@ -25,4 +25,11 @@ }) end) end + + if ENV["ADD_REQUEST_ON_ERROR"] == "true" + config.add_on_error(proc do |report| + report.request[:something] = "hello" + report.request[:params][:another_thing] = "hi" + end) + end end diff --git a/features/fixtures/rails4/app/config/initializers/bugsnag.rb b/features/fixtures/rails4/app/config/initializers/bugsnag.rb index baa9c5d3f..ca5b93ca7 100644 --- a/features/fixtures/rails4/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails4/app/config/initializers/bugsnag.rb @@ -25,4 +25,11 @@ }) end) end + + if ENV["ADD_REQUEST_ON_ERROR"] == "true" + config.add_on_error(proc do |report| + report.request[:something] = "hello" + report.request[:params][:another_thing] = "hi" + end) + end end diff --git a/features/fixtures/rails5/app/config/initializers/bugsnag.rb b/features/fixtures/rails5/app/config/initializers/bugsnag.rb index baa9c5d3f..258d13b59 100644 --- a/features/fixtures/rails5/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails5/app/config/initializers/bugsnag.rb @@ -7,14 +7,14 @@ config.auto_notify = ENV["BUGSNAG_AUTO_NOTIFY"] != "false" config.project_root = ENV["BUGSNAG_PROJECT_ROOT"] if ENV.include? "BUGSNAG_PROJECT_ROOT" config.ignore_classes << lambda { |ex| ex.class.to_s == ENV["BUGSNAG_IGNORE_CLASS"] } if ENV.include? "BUGSNAG_IGNORE_CLASS" - config.auto_capture_sessions = ENV["BUGSNAG_AUTO_CAPTURE_SESSIONS"] == "true" unless ENV["USE_DEFAULT_AUTO_CAPTURE_SESSIONS"] == "true" + config.auto_track_sessions = ENV["BUGSNAG_AUTO_CAPTURE_SESSIONS"] == "true" unless ENV["USE_DEFAULT_AUTO_CAPTURE_SESSIONS"] == "true" config.send_code = ENV["BUGSNAG_SEND_CODE"] != "false" config.send_environment = ENV["BUGSNAG_SEND_ENVIRONMENT"] == "true" config.meta_data_filters << 'filtered_parameter' if ENV["SQL_ONLY_BREADCRUMBS"] == "true" config.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - breadcrumb.ignore! unless breadcrumb.meta_data[:event_name] == "sql.active_record" && breadcrumb.meta_data[:name] == "User Load" + breadcrumb.ignore! unless breadcrumb.metadata[:event_name] == "sql.active_record" && breadcrumb.metadata[:name] == "User Load" end end @@ -25,4 +25,11 @@ }) end) end + + if ENV["ADD_REQUEST_ON_ERROR"] == "true" + config.add_on_error(proc do |report| + report.request[:something] = "hello" + report.request[:params][:another_thing] = "hi" + end) + end end diff --git a/features/fixtures/rails6/app/config/initializers/bugsnag.rb b/features/fixtures/rails6/app/config/initializers/bugsnag.rb index baa9c5d3f..ca5b93ca7 100644 --- a/features/fixtures/rails6/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails6/app/config/initializers/bugsnag.rb @@ -25,4 +25,11 @@ }) end) end + + if ENV["ADD_REQUEST_ON_ERROR"] == "true" + config.add_on_error(proc do |report| + report.request[:something] = "hello" + report.request[:params][:another_thing] = "hi" + end) + end end diff --git a/features/fixtures/rails_integrations/app/app/controllers/job_controller.rb b/features/fixtures/rails_integrations/app/app/controllers/job_controller.rb new file mode 100644 index 000000000..a480afd92 --- /dev/null +++ b/features/fixtures/rails_integrations/app/app/controllers/job_controller.rb @@ -0,0 +1,13 @@ +class JobController < ApplicationController + def working + WorkingJob.perform_later + + render json: { result: 'queued WorkingJob!' } + end + + def unhandled + UnhandledJob.perform_later + + render json: { result: 'queued UnhandledJob!' } + end +end diff --git a/features/fixtures/rails_integrations/app/app/jobs/working_job.rb b/features/fixtures/rails_integrations/app/app/jobs/working_job.rb new file mode 100644 index 000000000..3a191832c --- /dev/null +++ b/features/fixtures/rails_integrations/app/app/jobs/working_job.rb @@ -0,0 +1,23 @@ +class WorkingJob < ApplicationJob + self.queue_adapter = ENV['ACTIVE_JOB_QUEUE_ADAPTER'].to_sym + + def perform + do_stuff + + more_stuff + + success! + end + + def do_stuff + 'beep boop' + end + + def more_stuff + 'boop beep' + end + + def success! + 'yay' + end +end diff --git a/features/fixtures/rails_integrations/app/config/application.rb b/features/fixtures/rails_integrations/app/config/application.rb index 66b2dd2d0..f28492e96 100644 --- a/features/fixtures/rails_integrations/app/config/application.rb +++ b/features/fixtures/rails_integrations/app/config/application.rb @@ -32,7 +32,7 @@ class Application < Rails::Application # Only loads a smaller set of middleware suitable for API only apps. # Middleware like session, flash, cookies can be added back manually. # Skip views, helpers and assets when generating a new resource. - config.api_only = true + # config.api_only = true config.autoload_paths << Rails.root.join('app/workers') end diff --git a/features/fixtures/rails_integrations/app/config/environments/development.rb b/features/fixtures/rails_integrations/app/config/environments/development.rb index e0f5c3996..11cc198eb 100644 --- a/features/fixtures/rails_integrations/app/config/environments/development.rb +++ b/features/fixtures/rails_integrations/app/config/environments/development.rb @@ -44,4 +44,6 @@ # Use an evented file watcher to asynchronously detect changes in source code, # routes, locales, etc. This feature depends on the listen gem. config.file_watcher = ActiveSupport::EventedFileUpdateChecker + + config.hosts = [] end diff --git a/features/fixtures/rails_integrations/app/config/routes.rb b/features/fixtures/rails_integrations/app/config/routes.rb index c06383a17..6ffc2be67 100644 --- a/features/fixtures/rails_integrations/app/config/routes.rb +++ b/features/fixtures/rails_integrations/app/config/routes.rb @@ -1,3 +1,4 @@ Rails.application.routes.draw do - # For details on the DSL available within this file, see https://guides.rubyonrails.org/routing.html + get '/job/working', 'job#working' + get '/job/unhandled', 'job#unhandled' end diff --git a/features/fixtures/run-ruby-integrations b/features/fixtures/run-ruby-integrations new file mode 100755 index 000000000..46a8aea47 --- /dev/null +++ b/features/fixtures/run-ruby-integrations @@ -0,0 +1,120 @@ +#!/usr/bin/env ruby + +require "socket" +require "timeout" +require "pathname" + +DOCKER_DIRECTORY = Pathname.new(__dir__) +ROOT_DIRECTORY = DOCKER_DIRECTORY + "../.." +FIXTURE_DIRECTORY = DOCKER_DIRECTORY + "rails_integrations" + +raise "Fixture directory does not exist at: '#{FIXTURE_DIRECTORY}'" unless FIXTURE_DIRECTORY.exist? + +QUEUE_LIBRARY_COMMANDS = { + sidekiq: 'sidekiq', + resque: 'rake resque:work', + que: 'que --log-level debug --queue-name "*" ./config/environment.rb', + delayed_job: 'rake jobs:work', +} + +QUEUE_LIBRARY = ARGV.fetch(0, :sidekiq).to_sym + +raise "Invalid queue libarary '#{QUEUE_LIBRARY}'" unless QUEUE_LIBRARY_COMMANDS.key?(QUEUE_LIBRARY) + +def wait_for_port(port, max_attempts: 60, seconds_between_attempts: 1) + is_open = false + attempts = 0 + + until is_open || attempts > max_attempts + begin + attempts += 1 + + # add a timeout as sometimes TCPSocket will wait for ages before realising + # it can't connect - this is a local port so should be """instant""" + Timeout.timeout(2) do + TCPSocket.new("127.0.0.1", port).close + + # success! + is_open = true + end + rescue Timeout::Error, Errno::ECONNREFUSED, Errno::EHOSTUNREACH + # ignore timeouts and errors from the port being closed + + # wait between attempts to give the port some time to open + sleep(seconds_between_attempts) + end + end + + raise "Port #{port} not open in time!" unless is_open +end + +def run_in_shell(command, env: {}, background: false) + puts "running '#{command}' with env: #{env}, background: #{background}" + + if background + spawn(env, command) + else + system(env, command, exception: true) + end +end + +def run_docker_command(command, env: {}, **kwargs) + default_env = { "NETWORK_NAME" => "notwerk-norm", "ACTIVE_JOB_QUEUE_ADAPTER" => QUEUE_LIBRARY.to_s } + + run_in_shell(command, env: default_env.merge(env), **kwargs) +end + +# ensure we clean up after ourselves on exit +at_exit do + temp_bugsnag_lib = FIXTURE_DIRECTORY + "temp-bugsnag-lib" + temp_bugsnag_lib.rmtree if temp_bugsnag_lib.exist? + + # stop the docker compose stack + Dir.chdir(FIXTURE_DIRECTORY) do + run_docker_command("docker-compose down") + end +end + +# build the bugsnag gem and move it to the fixture directory +Dir.chdir(ROOT_DIRECTORY) do + run_in_shell("gem build bugsnag.gemspec -o bugsnag.gem") + run_in_shell("mv bugsnag.gem #{FIXTURE_DIRECTORY}") +end + +Dir.chdir(FIXTURE_DIRECTORY) do + # unpack the gem into the directory the dockerfile expects + run_in_shell("gem unpack bugsnag.gem --target temp-bugsnag-lib") + run_in_shell("rm bugsnag.gem") + + rails_pid = run_docker_command( + "docker-compose up --build rails_integrations", + env: { "RUBY_TEST_VERSION" => "2.7" }, + background: true + ) + + # wait for the container to finish building & starting + wait_for_port(3000) + + # setup and migrate the database + run_docker_command("docker-compose run rails_integrations bundle exec rake db:prepare") + run_docker_command("docker-compose run rails_integrations bundle exec rake db:migrate") + + # run the queue library in the background (not using '--detach' so we can see the logs) + queue_library_pid = run_docker_command( + "docker-compose run rails_integrations bundle exec #{QUEUE_LIBRARY_COMMANDS[QUEUE_LIBRARY]}", + background: true + ) + + # give the queue library some time to start before we print stuff, otherwise + # we'll print before the library does + sleep(5) + + puts "Everything is running!" + + # this will wait forever as the queue libraries won't exit on their own - quit with Ctrl+C + Process.wait(queue_library_pid) + + # the queue library has exited (because of Ctrl+C) so tell rails to stop too, + # otherwise you'll need to Ctrl+C twice and no one has time for that + Process.kill("TERM", rails_pid) +end diff --git a/features/plain_features/handled_errors.feature b/features/plain_features/handled_errors.feature index 88e98a7ff..ff97e32e9 100644 --- a/features/plain_features/handled_errors.feature +++ b/features/plain_features/handled_errors.feature @@ -7,6 +7,7 @@ Scenario: A rescued exception sends a report And the event "unhandled" is false And the event "severity" equals "warning" And the event "severityReason.type" equals "handledException" + And the event "device.time" is a timestamp And the exception "errorClass" equals "RuntimeError" And the "file" of stack frame 0 equals "/usr/src/app/handled/notify_exception.rb" And the "lineNumber" of stack frame 0 equals 6 @@ -18,6 +19,7 @@ Scenario: A notified string sends a report And the event "unhandled" is false And the event "severity" equals "warning" And the event "severityReason.type" equals "handledException" + And the event "device.time" is a timestamp And the exception "errorClass" equals "RuntimeError" And the "file" of the top non-bugsnag stackframe equals "/usr/src/app/handled/notify_string.rb" And the "lineNumber" of the top non-bugsnag stackframe equals 8 @@ -39,4 +41,5 @@ Scenario: A handled error can attach metadata in a block And the "lineNumber" of stack frame 0 equals 6 And the event "metaData.account.id" equals "1234abcd" And the event "metaData.account.name" equals "Acme Co" - And the event "metaData.account.support" is true \ No newline at end of file + And the event "metaData.account.support" is true + And the event "device.time" is a timestamp diff --git a/features/plain_features/unhandled_errors.feature b/features/plain_features/unhandled_errors.feature index 2b227dc73..6bca5787f 100644 --- a/features/plain_features/unhandled_errors.feature +++ b/features/plain_features/unhandled_errors.feature @@ -7,6 +7,7 @@ Scenario Outline: An unhandled error sends a report And the event "unhandled" is true And the event "severity" equals "error" And the event "severityReason.type" equals "unhandledException" + And the event "device.time" is a timestamp And the exception "errorClass" equals "" And the "file" of stack frame 0 equals "/usr/src/app/unhandled/.rb" And the "lineNumber" of stack frame 0 equals diff --git a/features/rails_features/integrations.feature b/features/rails_features/integrations.feature index 093d7ab72..6743a0bc6 100644 --- a/features/rails_features/integrations.feature +++ b/features/rails_features/integrations.feature @@ -128,7 +128,7 @@ Scenario: Sidekiq And the event "metaData.sidekiq.queue" equals "default" @rails_integrations -Scenario: Using Sidekiq as the Active Job queue adapter +Scenario: Using Sidekiq as the Active Job queue adapter for a job that raises When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "sidekiq" And I run "bundle exec sidekiq" in the rails app And I run "UnhandledJob.perform_later(1, yes: true)" with the rails runner @@ -150,7 +150,7 @@ Scenario: Using Sidekiq as the Active Job queue adapter And the event "metaData.sidekiq.queue" equals "default" @rails_integrations -Scenario: Using Rescue as the Active Job queue adapter +Scenario: Using Rescue as the Active Job queue adapter for a job that raises When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "resque" And I run "bundle exec rake resque:work" in the rails app And I run "UnhandledJob.perform_later(1, yes: true)" with the rails runner @@ -171,7 +171,7 @@ Scenario: Using Rescue as the Active Job queue adapter And the event "metaData.payload.args.0.queue_name" equals "default" @rails_integrations -Scenario: Using Que as the Active Job queue adapter +Scenario: Using Que as the Active Job queue adapter for a job that raises When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "que" And I run "bundle exec que -q default ./config/environment.rb" in the rails app And I run "UnhandledJob.perform_later(1, yes: true)" with the rails runner @@ -192,7 +192,7 @@ Scenario: Using Que as the Active Job queue adapter And the event "metaData.job.queue" equals "default" @rails_integrations -Scenario: Using Delayed Job as the Active Job queue adapter +Scenario: Using Delayed Job as the Active Job queue adapter for a job that raises When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "delayed_job" And I run the "jobs:work" rake task in the rails app And I run "UnhandledJob.perform_later(1, yes: true)" with the rails runner @@ -211,3 +211,35 @@ Scenario: Using Delayed Job as the Active Job queue adapter And the event "metaData.job.payload.arguments.0" equals 1 And the event "metaData.job.payload.arguments.1.yes" is true And the event "metaData.job.queue" equals "default" + +@rails_integrations +Scenario: Using Sidekiq as the Active Job queue adapter for a job that works + When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "sidekiq" + And I run "bundle exec sidekiq" in the rails app + And I run "WorkingJob.perform_later" with the rails runner + And I wait for 10 seconds + Then I should receive no requests + +@rails_integrations +Scenario: Using Rescue as the Active Job queue adapter for a job that works + When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "resque" + And I run "bundle exec rake resque:work" in the rails app + And I run "WorkingJob.perform_later" with the rails runner + And I wait for 10 seconds + Then I should receive no requests + +@rails_integrations +Scenario: Using Que as the Active Job queue adapter for a job that works + When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "que" + And I run "bundle exec que -q default ./config/environment.rb" in the rails app + And I run "WorkingJob.perform_later" with the rails runner + And I wait for 10 seconds + Then I should receive no requests + +@rails_integrations +Scenario: Using Delayed Job as the Active Job queue adapter for a job that works + When I set environment variable "ACTIVE_JOB_QUEUE_ADAPTER" to "delayed_job" + And I run the "jobs:work" rake task in the rails app + And I run "WorkingJob.perform_later" with the rails runner + And I wait for 10 seconds + Then I should receive no requests diff --git a/features/rails_features/request.feature b/features/rails_features/request.feature new file mode 100644 index 000000000..46e735ea0 --- /dev/null +++ b/features/rails_features/request.feature @@ -0,0 +1,52 @@ +Feature: Request data + +@rails3 @rails4 @rails5 @rails6 +Scenario: Request data is collected automatically + Given I start the rails service + When I navigate to the route "/unhandled/error?a=123&b=456" on the rails app + And I wait to receive a request + Then the request is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" + And the event "unhandled" is true + And the exception "errorClass" equals "NameError" + And the exception "message" starts with "undefined local variable or method `generate_unhandled_error' for # 0 ? c.call(breadcrumb) : c.call break if breadcrumb.ignore? @@ -259,6 +263,10 @@ def leave_breadcrumb(name, meta_data={}, type=Bugsnag::Breadcrumbs::MANUAL_BREAD # Return early if ignored return if breadcrumb.ignore? + # Run on_breadcrumb callbacks + configuration.on_breadcrumb_callbacks.call(breadcrumb) + return if breadcrumb.ignore? + # Validate again in case of callback alteration validator.validate(breadcrumb) @@ -293,6 +301,44 @@ def remove_on_error(callback) configuration.remove_on_error(callback) end + ## + # Add the given callback to the list of on_breadcrumb callbacks + # + # The on_breadcrumb callbacks will be called when a breadcrumb is left and + # are passed the {Breadcrumbs::Breadcrumb Breadcrumb} object + # + # Returning false from an on_breadcrumb callback will cause the breadcrumb + # to be ignored and will prevent any remaining callbacks from being called + # + # @param callback [Proc, Method, #call] + # @return [void] + def add_on_breadcrumb(callback) + configuration.add_on_breadcrumb(callback) + end + + ## + # Remove the given callback from the list of on_breadcrumb callbacks + # + # Note that this must be the same instance that was passed to + # {add_on_breadcrumb}, otherwise it will not be removed + # + # @param callback [Proc, Method, #call] + # @return [void] + def remove_on_breadcrumb(callback) + configuration.remove_on_breadcrumb(callback) + end + + ## + # Returns the current list of breadcrumbs + # + # This is a per-thread circular buffer, containing at most 'max_breadcrumbs' + # breadcrumbs + # + # @return [Bugsnag::Utility::CircularBuffer] + def breadcrumbs + configuration.breadcrumbs + end + ## # Returns the client's Cleaner object, or creates one if not yet created. # diff --git a/lib/bugsnag/breadcrumb_type.rb b/lib/bugsnag/breadcrumb_type.rb new file mode 100644 index 000000000..f8a103ccc --- /dev/null +++ b/lib/bugsnag/breadcrumb_type.rb @@ -0,0 +1,14 @@ +require "bugsnag/breadcrumbs/breadcrumbs" + +module Bugsnag + module BreadcrumbType + ERROR = Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE + LOG = Bugsnag::Breadcrumbs::LOG_BREADCRUMB_TYPE + MANUAL = Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE + NAVIGATION = Bugsnag::Breadcrumbs::NAVIGATION_BREADCRUMB_TYPE + PROCESS = Bugsnag::Breadcrumbs::PROCESS_BREADCRUMB_TYPE + REQUEST = Bugsnag::Breadcrumbs::REQUEST_BREADCRUMB_TYPE + STATE = Bugsnag::Breadcrumbs::STATE_BREADCRUMB_TYPE + USER = Bugsnag::Breadcrumbs::USER_BREADCRUMB_TYPE + end +end diff --git a/lib/bugsnag/breadcrumbs/breadcrumb.rb b/lib/bugsnag/breadcrumbs/breadcrumb.rb index 90d9533c9..a5ed2e16b 100644 --- a/lib/bugsnag/breadcrumbs/breadcrumb.rb +++ b/lib/bugsnag/breadcrumbs/breadcrumb.rb @@ -1,11 +1,13 @@ module Bugsnag::Breadcrumbs class Breadcrumb + # @deprecated Use {#message} instead # @return [String] the breadcrumb name attr_accessor :name # @return [String] the breadcrumb type attr_accessor :type + # @deprecated Use {#metadata} instead # @return [Hash, nil] metadata hash containing strings, numbers, or booleans, or nil attr_accessor :meta_data @@ -23,7 +25,7 @@ class Breadcrumb # @api private # # @param name [String] the breadcrumb name - # @param type [String] the breadcrumb type from Bugsnag::Breadcrumbs::VALID_BREADCRUMB_TYPES + # @param type [String] the breadcrumb type from Bugsnag::BreadcrumbType # @param meta_data [Hash, nil] a hash containing strings, numbers, or booleans, or nil # @param auto [Symbol] set to `:auto` if the breadcrumb is automatically generated def initialize(name, type, meta_data, auto) @@ -72,5 +74,36 @@ def to_h :timestamp => @timestamp.iso8601(3) } end + + # TODO: "message" and "metadata" can be simple attr_accessors when they + # replace "name" and "meta_data" + # NOTE: these are not aliases as YARD doesn't allow documenting the non-alias + # as deprecated without also marking the alias as deprecated + + # The breadcrumb message + # @!attribute message + # @return [String] + def message + @name + end + + # @param message [String] + # @return [void] + def message=(message) + @name = message + end + + # A Hash containing arbitrary metadata associated with this breadcrumb + # @!attribute metadata + # @return [Hash, nil] + def metadata + @meta_data + end + + # @param metadata [Hash, nil] + # @return [void] + def metadata=(metadata) + @meta_data = metadata + end end end diff --git a/lib/bugsnag/breadcrumbs/breadcrumbs.rb b/lib/bugsnag/breadcrumbs/breadcrumbs.rb index 69506d29d..b92b325cf 100644 --- a/lib/bugsnag/breadcrumbs/breadcrumbs.rb +++ b/lib/bugsnag/breadcrumbs/breadcrumbs.rb @@ -1,4 +1,5 @@ module Bugsnag::Breadcrumbs + # @deprecated Use {Bugsnag::BreadcrumbType} instead VALID_BREADCRUMB_TYPES = [ ERROR_BREADCRUMB_TYPE = "error", MANUAL_BREADCRUMB_TYPE = "manual", diff --git a/lib/bugsnag/breadcrumbs/on_breadcrumb_callback_list.rb b/lib/bugsnag/breadcrumbs/on_breadcrumb_callback_list.rb new file mode 100644 index 000000000..df7cc5add --- /dev/null +++ b/lib/bugsnag/breadcrumbs/on_breadcrumb_callback_list.rb @@ -0,0 +1,50 @@ +require "set" + +module Bugsnag::Breadcrumbs + class OnBreadcrumbCallbackList + def initialize(configuration) + @callbacks = Set.new + @mutex = Mutex.new + @configuration = configuration + end + + ## + # @param callback [Proc, Method, #call] + # @return [void] + def add(callback) + @mutex.synchronize do + @callbacks.add(callback) + end + end + + ## + # @param callback [Proc, Method, #call] + # @return [void] + def remove(callback) + @mutex.synchronize do + @callbacks.delete(callback) + end + end + + ## + # @param breadcrumb [Breadcrumb] + # @return [void] + def call(breadcrumb) + @callbacks.each do |callback| + begin + should_continue = callback.call(breadcrumb) + rescue StandardError => e + @configuration.warn("Error occurred in on_breadcrumb callback: '#{e}'") + @configuration.warn("on_breadcrumb callback stacktrace: #{e.backtrace.inspect}") + end + + # only stop if should_continue is explicity 'false' to allow callbacks + # to return 'nil' + if should_continue == false + breadcrumb.ignore! + break + end + end + end + end +end diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index ea8abc1ec..b7468cc33 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -12,6 +12,7 @@ require "bugsnag/middleware/breadcrumbs" require "bugsnag/utility/circular_buffer" require "bugsnag/breadcrumbs/breadcrumbs" +require "bugsnag/breadcrumbs/on_breadcrumb_callback_list" module Bugsnag class Configuration @@ -24,6 +25,7 @@ class Configuration attr_accessor :release_stage # A list of which release stages should cause notifications to be sent + # @deprecated Use {#enabled_release_stages} instead # @return [Array, nil] attr_accessor :notify_release_stages @@ -104,6 +106,7 @@ class Configuration attr_accessor :discard_classes # Whether Bugsnag should automatically record sessions + # @deprecated Use {#auto_track_sessions} instead # @return [Boolean] attr_accessor :auto_capture_sessions @@ -125,7 +128,8 @@ class Configuration attr_reader :enable_sessions # A list of strings indicating allowable automatic breadcrumb types - # @see Bugsnag::Breadcrumbs::VALID_BREADCRUMB_TYPES + # @deprecated Use {#enabled_breadcrumb_types} instead + # @see Bugsnag::BreadcrumbType # @return [Array] attr_accessor :enabled_automatic_breadcrumb_types @@ -141,10 +145,20 @@ class Configuration # @return [Regexp] attr_accessor :vendor_path + # The default context for all future events + # Setting this will disable automatic context setting + # @return [String, nil] + attr_accessor :context + # @api private # @return [Array] attr_reader :scopes_to_filter + # Expose on_breadcrumb_callbacks internally for Bugsnag.leave_breadcrumb + # @api private + # @return [Breadcrumbs::OnBreadcrumbCallbackList] + attr_reader :on_breadcrumb_callbacks + API_KEY_REGEX = /[0-9a-f]{32}/i THREAD_LOCAL_NAME = "bugsnag_req_data" @@ -193,6 +207,7 @@ def initialize # All valid breadcrumb types should be allowable initially self.enabled_automatic_breadcrumb_types = Bugsnag::Breadcrumbs::VALID_BREADCRUMB_TYPES.dup self.before_breadcrumb_callbacks = [] + @on_breadcrumb_callbacks = Breadcrumbs::OnBreadcrumbCallbackList.new(self) # Store max_breadcrumbs here instead of outputting breadcrumbs.max_items # to avoid infinite recursion when creating breadcrumb buffer @@ -389,15 +404,23 @@ def info(message) ## # Logs a warning level message # - # @param (see info) + # @param message [String, #to_s] The message to log def warn(message) logger.warn(PROG_NAME) { message } end + ## + # Logs an error level message + # + # @param message [String, #to_s] The message to log + def error(message) + logger.error(PROG_NAME) { message } + end + ## # Logs a debug level message # - # @param (see info) + # @param message [String, #to_s] The message to log def debug(message) logger.debug(PROG_NAME) { message } end @@ -426,9 +449,12 @@ def max_breadcrumbs=(new_max_breadcrumbs) end ## - # Returns the breadcrumb circular buffer + # Returns the current list of breadcrumbs # - # @return [Bugsnag::Utility::CircularBuffer] a thread based circular buffer containing breadcrumbs + # This is a per-thread circular buffer, containing at most 'max_breadcrumbs' + # breadcrumbs + # + # @return [Bugsnag::Utility::CircularBuffer] def breadcrumbs request_data[:breadcrumbs] ||= Bugsnag::Utility::CircularBuffer.new(@max_breadcrumbs) end @@ -503,6 +529,90 @@ def remove_on_error(callback) middleware.remove(callback) end + ## + # Add the given callback to the list of on_breadcrumb callbacks + # + # The on_breadcrumb callbacks will be called when a breadcrumb is left and + # are passed the {Breadcrumbs::Breadcrumb Breadcrumb} object + # + # Returning false from an on_breadcrumb callback will cause the breadcrumb + # to be ignored and will prevent any remaining callbacks from being called + # + # @param callback [Proc, Method, #call] + # @return [void] + def add_on_breadcrumb(callback) + @on_breadcrumb_callbacks.add(callback) + end + + ## + # Remove the given callback from the list of on_breadcrumb callbacks + # + # Note that this must be the same instance that was passed to + # {add_on_breadcrumb}, otherwise it will not be removed + # + # @param callback [Proc, Method, #call] + # @return [void] + def remove_on_breadcrumb(callback) + @on_breadcrumb_callbacks.remove(callback) + end + + ## + # Has the context been explicitly set? + # + # This is necessary to differentiate between the context not being set and + # the context being set to 'nil' explicitly + # + # @api private + # @return [Boolean] + def context_set? + defined?(@context) != nil + end + + # TODO: These methods can be a simple attr_accessor when they replace the + # methods they are aliasing + # NOTE: they are not aliases as YARD doesn't allow documenting the non-alias + # as deprecated without also marking the alias as deprecated + + # A list of which release stages should cause notifications to be sent + # @!attribute enabled_release_stages + # @return [Array, nil] + def enabled_release_stages + @notify_release_stages + end + + # @param release_stages [Array, nil] + # @return [void] + def enabled_release_stages=(release_stages) + @notify_release_stages = release_stages + end + + # A list of breadcrumb types that Bugsnag will collect automatically + # @!attribute enabled_breadcrumb_types + # @see Bugsnag::BreadcrumbType + # @return [Array] + def enabled_breadcrumb_types + @enabled_automatic_breadcrumb_types + end + + # @param breadcrumb_types [Array] + # @return [void] + def enabled_breadcrumb_types=(breadcrumb_types) + @enabled_automatic_breadcrumb_types = breadcrumb_types + end + + # Whether sessions should be tracked automatically + # @!attribute auto_track_sessions + # @return [Boolean] + def auto_track_sessions + @auto_capture_sessions + end + + # @param track_sessions [Boolean] + # @return [void] + def auto_track_sessions=(track_sessions) + @auto_capture_sessions = track_sessions + end + private attr_writer :scopes_to_filter diff --git a/lib/bugsnag/delivery/synchronous.rb b/lib/bugsnag/delivery/synchronous.rb index b5444ce5b..b0da7fde5 100644 --- a/lib/bugsnag/delivery/synchronous.rb +++ b/lib/bugsnag/delivery/synchronous.rb @@ -18,8 +18,8 @@ def deliver(url, body, configuration, options={}) # KLUDGE: Since we don't re-raise http exceptions, this breaks rspec raise if e.class.to_s == "RSpec::Expectations::ExpectationNotMetError" - configuration.warn("Notification to #{url} failed, #{e.inspect}") - configuration.warn(e.backtrace) + configuration.error("Unable to send information to Bugsnag (#{url}), #{e.inspect}") + configuration.error(e.backtrace) end end diff --git a/lib/bugsnag/delivery/thread_queue.rb b/lib/bugsnag/delivery/thread_queue.rb index 50423cedb..2b82b2212 100644 --- a/lib/bugsnag/delivery/thread_queue.rb +++ b/lib/bugsnag/delivery/thread_queue.rb @@ -31,8 +31,8 @@ def serialize_and_deliver(url, get_payload, configuration, options={}) begin payload = get_payload.call rescue StandardError => e - configuration.warn("Notification to #{url} failed, #{e.inspect}") - configuration.warn(e.backtrace) + configuration.error("Unable to send information to Bugsnag (#{url}), #{e.inspect}") + configuration.error(e.backtrace) end Synchronous.deliver(url, payload, configuration, options) unless payload.nil? diff --git a/lib/bugsnag/error.rb b/lib/bugsnag/error.rb new file mode 100644 index 000000000..d6e429edd --- /dev/null +++ b/lib/bugsnag/error.rb @@ -0,0 +1,25 @@ +module Bugsnag + class Error + # @return [String] the error's class name + attr_accessor :error_class + + # @return [String] the error's message + attr_accessor :error_message + + # @return [Hash] the error's processed stacktrace + attr_reader :stacktrace + + # @return [String] the type of error (always "ruby") + attr_accessor :type + + # @api private + TYPE = "ruby".freeze + + def initialize(error_class, error_message, stacktrace) + @error_class = error_class + @error_message = error_message + @stacktrace = stacktrace + @type = TYPE + end + end +end diff --git a/lib/bugsnag/event.rb b/lib/bugsnag/event.rb new file mode 100644 index 000000000..af42bb814 --- /dev/null +++ b/lib/bugsnag/event.rb @@ -0,0 +1,7 @@ +require "bugsnag/report" + +module Bugsnag + # For now Event is just an alias of Report. This points to the same object so + # any changes to Report will also affect Event + Event = Report +end diff --git a/lib/bugsnag/integrations/resque.rb b/lib/bugsnag/integrations/resque.rb index 502895cf3..560dcfb05 100644 --- a/lib/bugsnag/integrations/resque.rb +++ b/lib/bugsnag/integrations/resque.rb @@ -56,7 +56,7 @@ def save context = "#{class_name}@#{queue}" report.meta_data.merge!({ context: context, payload: metadata }) - report.context = context + report.automatic_context = context end end end diff --git a/lib/bugsnag/middleware/active_job.rb b/lib/bugsnag/middleware/active_job.rb index 5ff05ef74..b85e7f9f9 100644 --- a/lib/bugsnag/middleware/active_job.rb +++ b/lib/bugsnag/middleware/active_job.rb @@ -9,7 +9,7 @@ def call(report) if data report.add_tab(:active_job, data) - report.context = "#{data[:job_name]}@#{data[:queue]}" + report.automatic_context = "#{data[:job_name]}@#{data[:queue]}" end @bugsnag.call(report) diff --git a/lib/bugsnag/middleware/delayed_job.rb b/lib/bugsnag/middleware/delayed_job.rb index c82d50528..bcef91c25 100644 --- a/lib/bugsnag/middleware/delayed_job.rb +++ b/lib/bugsnag/middleware/delayed_job.rb @@ -30,7 +30,7 @@ def call(report) payload_data = construct_job_payload(job.payload_object) context = get_context(payload_data, job_data[:active_job]) - report.context = context unless context.nil? + report.automatic_context = context unless context.nil? job_data[:payload] = payload_data end diff --git a/lib/bugsnag/middleware/exception_meta_data.rb b/lib/bugsnag/middleware/exception_meta_data.rb index 5caa960a0..fddece6bc 100644 --- a/lib/bugsnag/middleware/exception_meta_data.rb +++ b/lib/bugsnag/middleware/exception_meta_data.rb @@ -16,6 +16,8 @@ def call(report) if exception.respond_to?(:bugsnag_context) context = exception.bugsnag_context + # note: this should set 'context' not 'automatic_context' as it's a + # user-supplied value report.context = context if context.is_a?(String) end diff --git a/lib/bugsnag/middleware/rack_request.rb b/lib/bugsnag/middleware/rack_request.rb index 6825ccf21..2fe6e365e 100644 --- a/lib/bugsnag/middleware/rack_request.rb +++ b/lib/bugsnag/middleware/rack_request.rb @@ -18,8 +18,8 @@ def call(report) client_ip = request.ip.to_s rescue SPOOF session = env["rack.session"] - # Set the context - report.context = "#{request.request_method} #{request.path}" + # Set the automatic context + report.automatic_context = "#{request.request_method} #{request.path}" # Set a sensible default for user_id report.user["id"] = request.ip diff --git a/lib/bugsnag/middleware/rails3_request.rb b/lib/bugsnag/middleware/rails3_request.rb index 16cc588d3..094350d27 100644 --- a/lib/bugsnag/middleware/rails3_request.rb +++ b/lib/bugsnag/middleware/rails3_request.rb @@ -15,8 +15,8 @@ def call(report) client_ip = env["action_dispatch.remote_ip"].to_s rescue SPOOF if params - # Set the context - report.context = "#{params[:controller]}##{params[:action]}" + # Set the automatic context + report.automatic_context = "#{params[:controller]}##{params[:action]}" # Augment the request tab report.add_tab(:request, { diff --git a/lib/bugsnag/middleware/rake.rb b/lib/bugsnag/middleware/rake.rb index ba6d6ca3b..379ff832f 100644 --- a/lib/bugsnag/middleware/rake.rb +++ b/lib/bugsnag/middleware/rake.rb @@ -16,7 +16,7 @@ def call(report) :arguments => task.arg_description }) - report.context ||= task.name + report.automatic_context ||= task.name end @bugsnag.call(report) diff --git a/lib/bugsnag/middleware/sidekiq.rb b/lib/bugsnag/middleware/sidekiq.rb index 453da2b2d..6adcdd7b7 100644 --- a/lib/bugsnag/middleware/sidekiq.rb +++ b/lib/bugsnag/middleware/sidekiq.rb @@ -10,7 +10,7 @@ def call(report) sidekiq = report.request_data[:sidekiq] if sidekiq report.add_tab(:sidekiq, sidekiq) - report.context ||= "#{sidekiq[:msg]['wrapped'] || sidekiq[:msg]['class']}@#{sidekiq[:msg]['queue']}" + report.automatic_context ||= "#{sidekiq[:msg]['wrapped'] || sidekiq[:msg]['class']}@#{sidekiq[:msg]['queue']}" end @bugsnag.call(report) end diff --git a/lib/bugsnag/report.rb b/lib/bugsnag/report.rb index 9fcb0ee86..79756788c 100644 --- a/lib/bugsnag/report.rb +++ b/lib/bugsnag/report.rb @@ -1,8 +1,10 @@ require "json" require "pathname" +require "bugsnag/error" require "bugsnag/stacktrace" module Bugsnag + # rubocop:todo Metrics/ClassLength class Report NOTIFIER_NAME = "Ruby Bugsnag Notifier" NOTIFIER_VERSION = Bugsnag::VERSION @@ -45,16 +47,13 @@ class Report # @return [Configuration] attr_accessor :configuration - # Additional context for this report - # @return [String, nil] - attr_accessor :context - # The delivery method that will be used for this report # @see Configuration#delivery_method # @return [Symbol] attr_accessor :delivery_method # The list of exceptions in this report + # @deprecated Use {#errors} instead # @return [Array] attr_accessor :exceptions @@ -72,10 +71,12 @@ class Report attr_accessor :grouping_hash # Arbitrary metadata attached to this report + # @deprecated Use {#metadata} instead # @return [Hash] attr_accessor :meta_data # The raw Exception instances for this report + # @deprecated Use {#original_error} instead # @see #exceptions # @return [Array] attr_accessor :raw_exceptions @@ -102,21 +103,35 @@ class Report # @return [Hash] attr_accessor :user + # A list of errors in this report + # @return [Array] + attr_reader :errors + + # The Exception instance this report was created for + # @return [Exception] + attr_reader :original_error + ## # Initializes a new report from an exception. def initialize(exception, passed_configuration, auto_notify=false) + # store the creation time for use as device.time + @created_at = Time.now.utc.iso8601(3) + @should_ignore = false @unhandled = auto_notify self.configuration = passed_configuration + @original_error = exception self.raw_exceptions = generate_raw_exceptions(exception) self.exceptions = generate_exception_list + @errors = generate_error_list self.api_key = configuration.api_key self.app_type = configuration.app_type self.app_version = configuration.app_version self.breadcrumbs = [] + self.context = configuration.context if configuration.context_set? self.delivery_method = configuration.delivery_method self.hostname = configuration.hostname self.runtime_versions = configuration.runtime_versions.dup @@ -125,8 +140,29 @@ def initialize(exception, passed_configuration, auto_notify=false) self.severity = auto_notify ? "error" : "warning" self.severity_reason = auto_notify ? {:type => UNHANDLED_EXCEPTION} : {:type => HANDLED_EXCEPTION} self.user = {} + + @metadata_delegate = Utility::MetadataDelegate.new end + ## + # Additional context for this report + # @!attribute context + # @return [String, nil] + def context + return @context if defined?(@context) + + @automatic_context + end + + attr_writer :context + + ## + # Context set automatically by Bugsnag uses this attribute, which prevents + # it from overwriting the user-supplied context + # @api private + # @return [String, nil] + attr_accessor :automatic_context + ## # Add a new metadata tab to this notification. # @@ -135,6 +171,8 @@ def initialize(exception, passed_configuration, auto_notify=false) # exists, this will be merged with the existing values. If a Hash is not # given, the value will be placed into the 'custom' tab # @return [void] + # + # @deprecated Use {#add_metadata} instead def add_tab(name, value) return if name.nil? @@ -153,6 +191,8 @@ def add_tab(name, value) # # @param name [String] # @return [void] + # + # @deprecated Use {#clear_metadata} instead def remove_tab(name) return if name.nil? @@ -175,7 +215,8 @@ def as_json context: context, device: { hostname: hostname, - runtimeVersions: runtime_versions + runtimeVersions: runtime_versions, + time: @created_at }, exceptions: exceptions, groupingHash: grouping_hash, @@ -257,6 +298,82 @@ def summary end end + # A Hash containing arbitrary metadata + # @!attribute metadata + # @return [Hash] + def metadata + @meta_data + end + + # @param metadata [Hash] + # @return [void] + def metadata=(metadata) + @meta_data = metadata + end + + ## + # Data from the current HTTP request. May be nil if no data has been recorded + # + # @return [Hash, nil] + def request + @meta_data[:request] + end + + ## + # Add values to metadata + # + # @overload add_metadata(section, data) + # Merges data into the given section of metadata + # @param section [String, Symbol] + # @param data [Hash] + # + # @overload add_metadata(section, key, value) + # Sets key to value in the given section of metadata. If the value is nil + # the key will be deleted + # @param section [String, Symbol] + # @param key [String, Symbol] + # @param value + # + # @return [void] + def add_metadata(section, key_or_data, *args) + @metadata_delegate.add_metadata(@meta_data, section, key_or_data, *args) + end + + ## + # Clear values from metadata + # + # @overload clear_metadata(section) + # Clears the given section of metadata + # @param section [String, Symbol] + # + # @overload clear_metadata(section, key) + # Clears the key in the given section of metadata + # @param section [String, Symbol] + # @param key [String, Symbol] + # + # @return [void] + def clear_metadata(section, *args) + @metadata_delegate.clear_metadata(@meta_data, section, *args) + end + + ## + # Set information about the current user + # + # Additional user fields can be added as metadata in a "user" section + # + # Setting a field to 'nil' will remove it from the user data + # + # @param id [String, nil] + # @param email [String, nil] + # @param name [String, nil] + # @return [void] + def set_user(id = nil, email = nil, name = nil) + new_user = { id: id, email: email, name: name } + new_user.reject! { |key, value| value.nil? } + + @user = new_user + end + private def generate_exception_list @@ -269,6 +386,12 @@ def generate_exception_list end end + def generate_error_list + exceptions.map do |exception| + Error.new(exception[:errorClass], exception[:message], exception[:stacktrace]) + end + end + def error_class(exception) # The "Class" check is for some strange exceptions like Timeout::Error # which throw the error class instead of an instance @@ -311,4 +434,5 @@ def generate_raw_exceptions(exception) exceptions end end + # rubocop:enable Metrics/ClassLength end diff --git a/lib/bugsnag/session_tracker.rb b/lib/bugsnag/session_tracker.rb index 65c0f98a5..810816f0d 100644 --- a/lib/bugsnag/session_tracker.rb +++ b/lib/bugsnag/session_tracker.rb @@ -35,7 +35,8 @@ def initialize # # This allows Bugsnag to track error rates for a release. def start_session - return unless Bugsnag.configuration.enable_sessions + return unless Bugsnag.configuration.enable_sessions && Bugsnag.configuration.should_notify_release_stage? + start_delivery_thread start_time = Time.now().utc().strftime('%Y-%m-%dT%H:%M:00') new_session = { @@ -83,7 +84,7 @@ def start_delivery_thread end end end - @delivery_thread = Concurrent::TimerTask.execute(execution_interval: 30) do + @delivery_thread = Concurrent::TimerTask.execute(execution_interval: 10) do if @session_counts.size > 0 send_sessions end @@ -106,11 +107,6 @@ def deliver(session_payload) return end - if !Bugsnag.configuration.should_notify_release_stage? - Bugsnag.configuration.debug("Not delivering sessions due to notify_release_stages :#{Bugsnag.configuration.notify_release_stages.inspect}") - return - end - body = { :notifier => { :name => Bugsnag::Report::NOTIFIER_NAME, diff --git a/lib/bugsnag/tasks/bugsnag.rake b/lib/bugsnag/tasks/bugsnag.rake index be214b3da..efc186e6f 100644 --- a/lib/bugsnag/tasks/bugsnag.rake +++ b/lib/bugsnag/tasks/bugsnag.rake @@ -7,7 +7,7 @@ namespace :bugsnag do raise RuntimeError.new("Bugsnag test exception") rescue => e Bugsnag.notify(e) do |report| - report.context = "rake#test_exception" + report.automatic_context = "rake#test_exception" end end end diff --git a/lib/bugsnag/utility/metadata_delegate.rb b/lib/bugsnag/utility/metadata_delegate.rb new file mode 100644 index 000000000..8b43c545d --- /dev/null +++ b/lib/bugsnag/utility/metadata_delegate.rb @@ -0,0 +1,102 @@ +module Bugsnag::Utility + # @api private + class MetadataDelegate + # nil is a valid metadata value, so we need a sentinel object so we can tell + # if the value parameter has been provided + NOT_PROVIDED = Object.new + + ## + # Add values to metadata + # + # @overload add_metadata(metadata, section, data) + # Merges data into the given section of metadata + # @param metadata [Hash] The metadata hash to operate on + # @param section [String, Symbol] + # @param data [Hash] + # + # @overload add_metadata(metadata, section, key, value) + # Sets key to value in the given section of metadata. If the value is nil + # the key will be deleted + # @param metadata [Hash] The metadata hash to operate on + # @param section [String, Symbol] + # @param key [String, Symbol] + # @param value + # + # @return [void] + def add_metadata(metadata, section, key_or_data, value = NOT_PROVIDED) + case value + when NOT_PROVIDED + merge_metadata(metadata, section, key_or_data) + when nil + clear_metadata(metadata, section, key_or_data) + else + overwrite_metadata(metadata, section, key_or_data, value) + end + end + + ## + # Clear values from metadata + # + # @overload clear_metadata(metadata, section) + # Clears the given section of metadata + # @param metadata [Hash] The metadata hash to operate on + # @param section [String, Symbol] + # + # @overload clear_metadata(metadata, section, key) + # Clears the key in the given section of metadata + # @param metadata [Hash] The metadata hash to operate on + # @param section [String, Symbol] + # @param key [String, Symbol] + # + # @return [void] + def clear_metadata(metadata, section, key = nil) + if key.nil? + metadata.delete(section) + elsif metadata[section] + metadata[section].delete(key) + end + end + + private + + ## + # Merge new metadata into the existing metadata + # + # Any keys with a 'nil' value in the new metadata will be deleted from the + # existing metadata + # + # @param existing_metadata [Hash] + # @param section [String, Symbol] + # @param new_metadata [Hash] + # @return [void] + def merge_metadata(existing_metadata, section, new_metadata) + return unless new_metadata.is_a?(Hash) + + existing_metadata[section] ||= {} + data = existing_metadata[section] + + new_metadata.each do |key, value| + if value.nil? + data.delete(key) + else + data[key] = value + end + end + end + + ## + # Overwrite the value in metadata's section & key + # + # @param metadata [Hash] + # @param section [String, Symbol] + # @param key [String, Symbol] + # @param value + # @return [void] + def overwrite_metadata(metadata, section, key, value) + return unless key.is_a?(String) || key.is_a?(Symbol) + + metadata[section] ||= {} + metadata[section][key] = value + end + end +end diff --git a/spec/breadcrumb_type_spec.rb b/spec/breadcrumb_type_spec.rb new file mode 100644 index 000000000..e1d75c747 --- /dev/null +++ b/spec/breadcrumb_type_spec.rb @@ -0,0 +1,23 @@ +require "spec_helper" + +require "bugsnag/breadcrumb_type" +require "bugsnag/breadcrumbs/breadcrumbs" + +describe Bugsnag::BreadcrumbType do + it "contains constants equivalent to the breadcrumb types defined in Bugsnag::Breadcrumbs" do + expect(Bugsnag::BreadcrumbType::ERROR).to eq(Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE) + expect(Bugsnag::BreadcrumbType::LOG).to eq(Bugsnag::Breadcrumbs::LOG_BREADCRUMB_TYPE) + expect(Bugsnag::BreadcrumbType::MANUAL).to eq(Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE) + expect(Bugsnag::BreadcrumbType::NAVIGATION).to eq(Bugsnag::Breadcrumbs::NAVIGATION_BREADCRUMB_TYPE) + expect(Bugsnag::BreadcrumbType::PROCESS).to eq(Bugsnag::Breadcrumbs::PROCESS_BREADCRUMB_TYPE) + expect(Bugsnag::BreadcrumbType::REQUEST).to eq(Bugsnag::Breadcrumbs::REQUEST_BREADCRUMB_TYPE) + expect(Bugsnag::BreadcrumbType::STATE).to eq(Bugsnag::Breadcrumbs::STATE_BREADCRUMB_TYPE) + expect(Bugsnag::BreadcrumbType::USER).to eq(Bugsnag::Breadcrumbs::USER_BREADCRUMB_TYPE) + end + + it "defines the same number of breadcrumb type constants" do + old_types = Bugsnag::Breadcrumbs.constants.select { |type| type.to_s.end_with?("_BREADCRUMB_TYPE") } + + expect(Bugsnag::BreadcrumbType.constants.length).to eq(old_types.length) + end +end diff --git a/spec/breadcrumbs/breadcrumb_spec.rb b/spec/breadcrumbs/breadcrumb_spec.rb index eb79f2c0f..b4bd43019 100644 --- a/spec/breadcrumbs/breadcrumb_spec.rb +++ b/spec/breadcrumbs/breadcrumb_spec.rb @@ -10,6 +10,19 @@ breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new("my message", nil, nil, nil) expect(breadcrumb.name).to eq("my message") + expect(breadcrumb.message).to eq("my message") + end + + it "can be accessed as #message" do + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new("my message", nil, nil, nil) + + breadcrumb.message = "my other message" + expect(breadcrumb.message).to eq("my other message") + expect(breadcrumb.name).to eq("my other message") + + breadcrumb.name = "another message" + expect(breadcrumb.message).to eq("another message") + expect(breadcrumb.name).to eq("another message") end end @@ -26,6 +39,19 @@ breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new(nil, nil, {:a => 1, :b => 2}, nil) expect(breadcrumb.meta_data).to eq({:a => 1, :b => 2}) + expect(breadcrumb.metadata).to eq({:a => 1, :b => 2}) + end + + it "can be accessed as #metadata" do + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new(nil, nil, { a: 1, b: 2 }, nil) + + breadcrumb.metadata = { c: 3 } + expect(breadcrumb.meta_data).to eq({ c: 3 }) + expect(breadcrumb.metadata).to eq({ c: 3 }) + + breadcrumb.meta_data = { d: 4 } + expect(breadcrumb.meta_data).to eq({ d: 4 }) + expect(breadcrumb.metadata).to eq({ d: 4 }) end end diff --git a/spec/breadcrumbs/on_breadcrumb_callback_list_spec.rb b/spec/breadcrumbs/on_breadcrumb_callback_list_spec.rb new file mode 100644 index 000000000..6011124ab --- /dev/null +++ b/spec/breadcrumbs/on_breadcrumb_callback_list_spec.rb @@ -0,0 +1,276 @@ +require "spec_helper" + +require "bugsnag/breadcrumbs/on_breadcrumb_callback_list" + +RSpec.describe Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList do + it "can add callbacks to its list" do + callback = spy('callback') + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) + list.add(callback) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(callback).to have_received(:call).with(breadcrumb) + expect(breadcrumb.ignore?).to be(false) + end + + it "can remove callbacks to its list" do + callback = spy('callback') + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) + list.add(callback) + list.remove(callback) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(callback).not_to have_received(:call) + expect(breadcrumb.ignore?).to be(false) + end + + it "won't remove a callback that isn't the same instance" do + callback1 = spy('callback1') + callback2 = spy('callback2') + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) + + # note: adding callback1 but removing callback2 + list.add(callback1) + list.remove(callback2) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(callback1).to have_received(:call).with(breadcrumb) + expect(callback2).not_to have_received(:call) + expect(breadcrumb.ignore?).to be(false) + end + + it "calls callbacks in the order they were added" do + calls = [] + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) + + list.add(proc { calls << 1 }) + list.add(proc { calls << 2 }) + list.add(proc { calls << 3 }) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(calls).to eq([1, 2, 3]) + expect(breadcrumb.ignore?).to be(false) + end + + it "ignores the breadcrumb if a callback returns false" do + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) + + list.add(proc { false }) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(breadcrumb.ignore?).to be(true) + end + + it "does not ignore the breadcrumb if a callback returns nil" do + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) + + list.add(proc { nil }) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(breadcrumb.ignore?).to be(false) + end + + it "supports Method objects as callbacks" do + class ArbitraryClassMethod + def self.arbitrary_name(breadcrumb) + breadcrumb.metadata[:abc] = 123 + end + end + + class ArbitraryInstanceMethod + def arbitrary_name(breadcrumb) + breadcrumb.metadata[:xyz] = 789 + end + end + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) + + list.add(ArbitraryClassMethod.method(:arbitrary_name)) + + xyz = ArbitraryInstanceMethod.new + list.add(xyz.method(:arbitrary_name)) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(breadcrumb.metadata).to eq({ abc: 123, xyz: 789 }) + end + + it "allows removing Method objects as callbacks" do + class ArbitraryClassMethod + def self.arbitrary_name(breadcrumb) + breadcrumb.metadata[:abc] = 123 + end + end + + class ArbitraryInstanceMethod + def arbitrary_name(breadcrumb) + breadcrumb.metadata[:xyz] = 789 + end + end + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) + + list.add(ArbitraryClassMethod.method(:arbitrary_name)) + list.remove(ArbitraryClassMethod.method(:arbitrary_name)) + + xyz = ArbitraryInstanceMethod.new + list.add(xyz.method(:arbitrary_name)) + list.remove(xyz.method(:arbitrary_name)) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(breadcrumb.metadata).to eq({}) + end + + it "supports arbitrary objects that respond to #call as callbacks" do + class RespondsToCallAsClassMethod + def self.call(breadcrumb) + breadcrumb.metadata[:abc] = 123 + end + end + + class RespondsToCallAsInstanceMethod + def call(breadcrumb) + breadcrumb.metadata[:xyz] = 789 + end + end + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) + + list.add(RespondsToCallAsClassMethod) + list.add(RespondsToCallAsInstanceMethod.new) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(breadcrumb.metadata).to eq({ abc: 123, xyz: 789 }) + end + + it "supports removing arbitrary objects that respond to #call as callbacks" do + class RespondsToCallAsClassMethod + def self.call(breadcrumb) + breadcrumb.metadata[:abc] = 123 + end + end + + class RespondsToCallAsInstanceMethod + def call(breadcrumb) + breadcrumb.metadata[:xyz] = 789 + end + end + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) + + list.add(RespondsToCallAsClassMethod) + list.remove(RespondsToCallAsClassMethod) + + instance = RespondsToCallAsInstanceMethod.new + list.add(instance) + list.remove(instance) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(breadcrumb.metadata).to eq({}) + end + + it "works when accessed concurrently" do + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) + + list.add(proc do |breadcrumb| + breadcrumb.metadata[:numbers] = [] + end) + + NUMBER_OF_THREADS = 20 + + threads = NUMBER_OF_THREADS.times.map do |i| + Thread.new do + list.add(proc do |breadcrumb| + breadcrumb.metadata[:numbers].push(i) + end) + end + end + + threads.shuffle.each(&:join) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + list.call(breadcrumb) + + # sort the numbers as they will be out of order but that doesn't matter as + # long as every number is present + expect(breadcrumb.metadata[:numbers].sort).to eq((0...NUMBER_OF_THREADS).to_a) + end + + it "logs errors thrown in callbacks" do + logger = spy('logger') + Bugsnag.configuration.logger = logger + + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) + error = RuntimeError.new('Oh no!') + + list.add(proc { raise error }) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(breadcrumb.ignore?).to be(false) + + message_index = 0 + expected_messages = [ + /^Error occurred in on_breadcrumb callback: 'Oh no!'$/, + /^on_breadcrumb callback stacktrace:/ + ] + + expect(logger).to have_received(:warn).with("[Bugsnag]").twice do |&block| + expect(block.call).to match(expected_messages[message_index]) + message_index += 1 + end + end + + it "calls subsequent callbacks after an error is raised" do + list = Bugsnag::Breadcrumbs::OnBreadcrumbCallbackList.new(Bugsnag.configuration) + calls = [] + + list.add(proc { calls << 1 }) + list.add(proc { calls << 2 }) + list.add(proc { raise 'ab' }) + list.add(proc { calls << 4 }) + list.add(proc { calls << 5 }) + + breadcrumb = Bugsnag::Breadcrumbs::Breadcrumb.new('name', 'type', {}, nil) + + list.call(breadcrumb) + + expect(calls).to eq([1, 2, 4, 5]) + expect(breadcrumb.ignore?).to be(false) + end +end diff --git a/spec/bugsnag_spec.rb b/spec/bugsnag_spec.rb index 15a16065a..7bd5dd7f7 100644 --- a/spec/bugsnag_spec.rb +++ b/spec/bugsnag_spec.rb @@ -323,104 +323,195 @@ module Kernel }) end - it "runs callbacks before leaving" do - Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - breadcrumb.meta_data = { - :callback => true - } + describe "before_breadcrumb_callbacks" do + it "runs callbacks before leaving" do + Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| + breadcrumb.meta_data = { + :callback => true + } + end + Bugsnag.leave_breadcrumb("TestName") + expect(breadcrumbs.to_a.size).to eq(1) + expect(breadcrumbs.first.to_h).to match({ + :name => "TestName", + :type => Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE, + :metaData => { + :callback => true + }, + :timestamp => match(timestamp_regex) + }) end - Bugsnag.leave_breadcrumb("TestName") - expect(breadcrumbs.to_a.size).to eq(1) - expect(breadcrumbs.first.to_h).to match({ - :name => "TestName", - :type => Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE, - :metaData => { - :callback => true - }, - :timestamp => match(timestamp_regex) - }) - end - it "validates after callbacks" do - Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - breadcrumb.meta_data = { - :int => 1, - :array => [1, 2, 3], - :hash => { - :a => 1, - :b => 2 + it "validates after callbacks" do + Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| + breadcrumb.meta_data = { + :int => 1, + :array => [1, 2, 3], + :hash => { + :a => 1, + :b => 2 + } } - } - breadcrumb.type = "Not a real type" - breadcrumb.name = "123123123123123123123123123123456456456456456" + breadcrumb.type = "Not a real type" + breadcrumb.name = "123123123123123123123123123123456456456456456" + end + + Bugsnag.leave_breadcrumb("TestName") + + expect(breadcrumbs.to_a.size).to eq(1) + expect(breadcrumbs.first.to_h).to match({ + :name => "123123123123123123123123123123456456456456456", + :type => Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE, + :metaData => { + :int => 1, + :array => [1, 2, 3], + :hash => { + :a => 1, + :b => 2 + } + }, + :timestamp => match(timestamp_regex) + }) end - Bugsnag.leave_breadcrumb("TestName") + it "doesn't add when ignored by the validator" do + Bugsnag.configuration.enabled_automatic_breadcrumb_types = [] + Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE, :auto) + expect(breadcrumbs.to_a.size).to eq(0) + end - expect(breadcrumbs.to_a.size).to eq(1) - expect(breadcrumbs.first.to_h).to match({ - :name => "123123123123123123123123123123456456456456456", - :type => Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE, - :metaData => { - :int => 1, - :array => [1, 2, 3], - :hash => { - :a => 1, - :b => 2 - } - }, - :timestamp => match(timestamp_regex) - }) - end + it "doesn't add if ignored in a callback" do + Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| + breadcrumb.ignore! + end + Bugsnag.leave_breadcrumb("TestName") + expect(breadcrumbs.to_a.size).to eq(0) + end - it "doesn't add when ignored by the validator" do - Bugsnag.configuration.enabled_automatic_breadcrumb_types = [] - Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE, :auto) - expect(breadcrumbs.to_a.size).to eq(0) - end + it "doesn't add when ignored after the callbacks" do + Bugsnag.configuration.enabled_automatic_breadcrumb_types = [ + Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE + ] + Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| + breadcrumb.type = Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE + end + Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE, :auto) + expect(breadcrumbs.to_a.size).to eq(0) + end - it "doesn't add if ignored in a callback" do - Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - breadcrumb.ignore! + it "doesn't call callbacks if ignored early" do + Bugsnag.configuration.enabled_automatic_breadcrumb_types = [] + Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| + fail "This shouldn't be called" + end + Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE, :auto) end - Bugsnag.leave_breadcrumb("TestName") - expect(breadcrumbs.to_a.size).to eq(0) - end - it "doesn't add when ignored after the callbacks" do - Bugsnag.configuration.enabled_automatic_breadcrumb_types = [ - Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE - ] - Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - breadcrumb.type = Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE + it "doesn't continue to call callbacks if ignored in them" do + Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| + breadcrumb.ignore! + end + Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| + fail "This shouldn't be called" + end + Bugsnag.leave_breadcrumb("TestName") end - Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::Breadcrumbs::MANUAL_BREADCRUMB_TYPE, :auto) - expect(breadcrumbs.to_a.size).to eq(0) end - it "doesn't call callbacks if ignored early" do - Bugsnag.configuration.enabled_automatic_breadcrumb_types = [] - Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - fail "This shouldn't be called" + describe "on_breadcrumb callbacks" do + it "runs callbacks when a breadcrumb is left" do + Bugsnag.add_on_breadcrumb(proc do |breadcrumb| + breadcrumb.metadata = { callback: true } + end) + + Bugsnag.leave_breadcrumb("TestName") + + expect(breadcrumbs.to_a.size).to eq(1) + expect(breadcrumbs.first.to_h).to match({ + name: "TestName", + type: Bugsnag::BreadcrumbType::MANUAL, + metaData: { callback: true }, + timestamp: match(timestamp_regex) + }) end - Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::Breadcrumbs::ERROR_BREADCRUMB_TYPE, :auto) - end - it "doesn't continue to call callbacks if ignored in them" do - Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - breadcrumb.ignore! + it "validates any changes made in a callback" do + Bugsnag.add_on_breadcrumb(proc do |breadcrumb| + breadcrumb.metadata = { abc: 123, xyz: { a: 1, b: 2 } } + + breadcrumb.type = "Not a real type" + breadcrumb.name = "123123123123123123123123123123456456456456456" + end) + + Bugsnag.leave_breadcrumb("TestName") + + expect(breadcrumbs.to_a.size).to eq(1) + expect(breadcrumbs.first.to_h).to match({ + name: "123123123123123123123123123123456456456456456", + type: Bugsnag::BreadcrumbType::MANUAL, + metaData: { abc: 123, xyz: { a: 1, b: 2 } }, + timestamp: match(timestamp_regex) + }) end - Bugsnag.configuration.before_breadcrumb_callbacks << Proc.new do |breadcrumb| - fail "This shouldn't be called" + + it "doesn't add the breadcrumb when ignored due to enabled_breadcrumb_types" do + Bugsnag.configure do |config| + config.enabled_breadcrumb_types = [Bugsnag::BreadcrumbType::MANUAL] + + config.add_on_breadcrumb(proc do |breadcrumb| + breadcrumb.type = Bugsnag::BreadcrumbType::ERROR + end) + end + + Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::BreadcrumbType::MANUAL, :auto) + + expect(breadcrumbs.to_a).to be_empty + end + + it "stops calling callbacks if the breadcrumb is ignored in them" do + callback1 = spy('callback1') + callback2 = spy('callback2') + + Bugsnag.configure do |config| + config.add_on_breadcrumb(callback1) + config.add_on_breadcrumb(proc { false }) + config.add_on_breadcrumb(callback2) + end + + Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::BreadcrumbType::ERROR, :auto) + + expect(callback1).to have_received(:call) + expect(callback2).not_to have_received(:call) + end + + it "continues calling callbacks after a callback raises" do + callback1 = spy('callback1') + callback2 = spy('callback2') + + Bugsnag.configure do |config| + config.add_on_breadcrumb(callback1) + config.add_on_breadcrumb(proc { raise 'uh oh' }) + config.add_on_breadcrumb(callback2) + end + + Bugsnag.leave_breadcrumb("TestName", {}, Bugsnag::BreadcrumbType::ERROR, :auto) + + expect(callback1).to have_received(:call) + expect(callback2).to have_received(:call) + expect(breadcrumbs.to_a.first.to_h).to match({ + name: "TestName", + type: Bugsnag::BreadcrumbType::ERROR, + metaData: {}, + timestamp: match(timestamp_regex) + }) end - Bugsnag.leave_breadcrumb("TestName") end end describe "request headers" do it "Bugsnag-Sent-At should use the current time" do fake_now = Time.gm(2020, 1, 2, 3, 4, 5, 123456) - expect(Time).to receive(:now).at_most(5).times.and_return(fake_now) + expect(Time).to receive(:now).at_most(6).times.and_return(fake_now) Bugsnag.notify(BugsnagTestException.new("It crashed")) @@ -430,4 +521,10 @@ module Kernel } end end + + describe "#breadcrumbs" do + it "returns the configuration's breadcrumb buffer" do + expect(Bugsnag.breadcrumbs).to be(Bugsnag.configuration.breadcrumbs) + end + end end diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index b4c8acd1c..98b95957c 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -61,6 +61,17 @@ end end + describe "context" do + it "should default to nil" do + expect(subject.context).to be_nil + end + + it "should be settable" do + subject.context = "test" + expect(subject.context).to eq("test") + end + end + describe "#notify_endpoint" do it "defaults to DEFAULT_NOTIFY_ENDPOINT" do expect(subject.notify_endpoint).to eq(Bugsnag::Configuration::DEFAULT_NOTIFY_ENDPOINT) @@ -87,6 +98,22 @@ end end + describe "#auto_track_sessions" do + it "defaults to true" do + expect(subject.auto_track_sessions).to eq(true) + end + + it "shares a backing boolean with 'auto_capture_sessions'" do + subject.auto_track_sessions = false + expect(subject.auto_track_sessions).to eq(false) + expect(subject.auto_capture_sessions).to eq(false) + + subject.auto_capture_sessions = true + expect(subject.auto_track_sessions).to eq(true) + expect(subject.auto_capture_sessions).to eq(true) + end + end + describe "#enable_sessions" do it "defaults to true" do expect(subject.enable_sessions).to eq(true) @@ -230,40 +257,21 @@ end describe "logger" do - class TestLogger - attr_accessor :logs - - def initialize - @logs = [] - end - - def log(level, name, &block) - message = block.call - @logs << { - :level => level, - :name => name, - :message => message - } + before do + @output = StringIO.new + @formatter = proc do |severity, _datetime, progname, message| + "#{progname} #{severity}: #{message}" end - def info(name, &block) - log('info', name, &block) - end + logger = Logger.new(@output) + logger.formatter = @formatter - def warn(name, &block) - log('warning', name, &block) - end - - def debug(name, &block) - log('debug', name, &block) - end + Bugsnag.configuration.logger = logger end - before do - @logger = TestLogger.new - Bugsnag.configure do |bugsnag| - bugsnag.logger = @logger - end + def output_lines + @output.rewind # always read from the start of output + @output.readlines.map(&:chomp) # old rubies don't support `readlines(chomp: true)` end context "using configure" do @@ -271,35 +279,31 @@ def debug(name, &block) Bugsnag.configuration.api_key = nil Bugsnag.instance_variable_set("@key_warning", nil) ENV['BUGSNAG_API_KEY'] = nil - expect(@logger.logs.size).to eq(0) + expect(output_lines).to be_empty end context "API key is not specified" do it "skips logging a warning if validate_api_key is false" do Bugsnag.configure(false) - expect(@logger.logs.size).to eq(0) + expect(output_lines).to be_empty end it "logs a warning by default" do Bugsnag.configure - expect(@logger.logs.size).to eq(1) - log = @logger.logs.first - expect(log).to eq({ - :level => "warning", - :name => "[Bugsnag]", - :message => "No valid API key has been set, notifications will not be sent" - }) + + expect(output_lines.length).to be(1) + expect(output_lines.first).to eq( + '[Bugsnag] WARN: No valid API key has been set, notifications will not be sent' + ) end it "logs a warning if validate_api_key is true" do Bugsnag.configure(true) - expect(@logger.logs.size).to eq(1) - log = @logger.logs.first - expect(log).to eq({ - :level => "warning", - :name => "[Bugsnag]", - :message => "No valid API key has been set, notifications will not be sent" - }) + + expect(output_lines.length).to be(1) + expect(output_lines.first).to eq( + '[Bugsnag] WARN: No valid API key has been set, notifications will not be sent' + ) end end @@ -308,64 +312,57 @@ def debug(name, &block) Bugsnag.configure do |config| config.api_key = 'd57a2472bd130ac0ab0f52715bbdc600' end - expect(@logger.logs.size).to eq(0) + + expect(output_lines).to be_empty end it "logs a warning if the configured API key is invalid" do Bugsnag.configure do |config| config.api_key = 'WARNING: not a real key' end - expect(@logger.logs.size).to eq(1) - log = @logger.logs.first - expect(log).to eq({ - :level => "warning", - :name => "[Bugsnag]", - :message => "No valid API key has been set, notifications will not be sent" - }) + + expect(output_lines.length).to be(1) + expect(output_lines.first).to eq( + '[Bugsnag] WARN: No valid API key has been set, notifications will not be sent' + ) end end end it "should log info messages to the set logger" do - expect(@logger.logs.size).to eq(0) + expect(output_lines).to be_empty + Bugsnag.configuration.info("Info message") - expect(@logger.logs.size).to eq(1) - log = @logger.logs.first - expect(log).to eq({ - :level => "info", - :name => "[Bugsnag]", - :message => "Info message" - }) + + expect(output_lines.length).to be(1) + expect(output_lines.first).to eq('[Bugsnag] INFO: Info message') end it "should log warning messages to the set logger" do - expect(@logger.logs.size).to eq(0) + expect(output_lines).to be_empty + Bugsnag.configuration.warn("Warning message") - expect(@logger.logs.size).to eq(1) - log = @logger.logs.first - expect(log).to eq({ - :level => "warning", - :name => "[Bugsnag]", - :message => "Warning message" - }) + + expect(output_lines.length).to be(1) + expect(output_lines.first).to eq('[Bugsnag] WARN: Warning message') + end + + it "should log error messages to the set logger" do + expect(output_lines).to be_empty + + Bugsnag.configuration.error("Error message") + + expect(output_lines.length).to be(1) + expect(output_lines.first).to eq('[Bugsnag] ERROR: Error message') end it "should log debug messages to the set logger" do - expect(@logger.logs.size).to eq(0) + expect(output_lines).to be_empty + Bugsnag.configuration.debug("Debug message") - expect(@logger.logs.size).to eq(1) - log = @logger.logs.first - expect(log).to eq({ - :level => "debug", - :name => "[Bugsnag]", - :message => "Debug message" - }) - end - after do - Bugsnag.configure do |bugsnag| - bugsnag.logger = Logger.new(StringIO.new) - end + expect(output_lines.length).to be(1) + expect(output_lines.first).to eq('[Bugsnag] DEBUG: Debug message') end end @@ -443,6 +440,29 @@ def debug(name, &block) end end + describe "#enabled_breadcrumb_types" do + it "defaults to Bugsnag::Breadcrumbs::VALID_BREADCRUMB_TYPES" do + expect(subject.enabled_breadcrumb_types).to eq(Bugsnag::Breadcrumbs::VALID_BREADCRUMB_TYPES) + end + + it "is an editable array" do + subject.enabled_breadcrumb_types << "Some custom type" + expect(subject.enabled_breadcrumb_types).to include("Some custom type") + end + + it "shares a backing array with 'enabled_automatic_breadcrumb_types'" do + expect(subject.enabled_breadcrumb_types).to be(subject.enabled_automatic_breadcrumb_types) + + subject.enabled_breadcrumb_types = [1, 2, 3] + expect(subject.enabled_breadcrumb_types).to eq([1, 2, 3]) + expect(subject.enabled_automatic_breadcrumb_types).to eq([1, 2, 3]) + + subject.enabled_automatic_breadcrumb_types = [4, 5, 6] + expect(subject.enabled_breadcrumb_types).to eq([4, 5, 6]) + expect(subject.enabled_automatic_breadcrumb_types).to eq([4, 5, 6]) + end + end + describe "#before_breadcrumb_callbacks" do it "initially returns an empty array" do expect(subject.before_breadcrumb_callbacks).to eq([]) diff --git a/spec/integrations/rack_spec.rb b/spec/integrations/rack_spec.rb index 57847efa4..b23e5fedb 100644 --- a/spec/integrations/rack_spec.rb +++ b/spec/integrations/rack_spec.rb @@ -92,135 +92,114 @@ class Request end end + after do + Object.send(:remove_const, :Rack) if @mocked_rack + end + it "correctly redacts from url and referer any value indicated by meta_data_filters" do - callback = double rack_env = { - :env => true, - :HTTP_REFERER => "https://bugsnag.com/about?email=hello@world.com&another_param=thing", - "rack.session" => { - :session => true - } + env: true, + HTTP_REFERER: "https://bugsnag.com/about?email=hello@world.com&another_param=thing", + "rack.session" => { session: true } } rack_request = double - rack_params = { - :param => 'test' - } allow(rack_request).to receive_messages( - :params => rack_params, - :ip => "rack_ip", - :request_method => "TEST", - :path => "/TEST_PATH", - :scheme => "http", - :host => "test_host", - :port => 80, - :referer => "https://bugsnag.com/about?email=hello@world.com&another_param=thing", - :fullpath => "/TEST_PATH?email=hello@world.com&another_param=thing" + params: { param: 'test' }, + ip: "rack_ip", + request_method: "TEST", + path: "/TEST_PATH", + scheme: "http", + host: "test_host", + port: 80, + referer: "https://bugsnag.com/about?email=hello@world.com&another_param=thing", + fullpath: "/TEST_PATH?email=hello@world.com&another_param=thing" ) + expect(::Rack::Request).to receive(:new).with(rack_env).and_return(rack_request) - # modify rack_env to include redacted referer - report = double("Bugsnag::Report") - allow(report).to receive(:request_data).and_return({ - :rack_env => rack_env - }) - expect(report).to receive(:context=).with("TEST /TEST_PATH") - expect(report).to receive(:user).and_return({}) - - config = Bugsnag.configuration - config.send_environment = true - config.meta_data_filters = ['email'] - - allow(report).to receive(:configuration).and_return(config) - expect(report).to receive(:add_tab).once.with(:request, { - :url => "http://test_host/TEST_PATH?email=[FILTERED]&another_param=thing", - :httpMethod => "TEST", - :params => rack_params, - :referer => "https://bugsnag.com/about?email=[FILTERED]&another_param=thing", - :clientIp => "rack_ip", - :headers => { - "Referer" => "https://bugsnag.com/about?email=[FILTERED]&another_param=thing" - } - }) - # rack_env["HTTP_REFERER"] = "https://bugsnag.com/about?email=[FILTERED]&another_param=thing" - expect(report).to receive(:add_tab).once.with(:environment, rack_env) - expect(report).to receive(:add_tab).once.with(:session, { - :session => true - }) + Bugsnag.configure do |config| + config.send_environment = true + config.meta_data_filters = ['email'] + config.request_data[:rack_env] = rack_env + end + report = Bugsnag::Report.new(RuntimeError.new('abc'), Bugsnag.configuration) + + callback = double expect(callback).to receive(:call).with(report) middleware = Bugsnag::Middleware::RackRequest.new(callback) middleware.call(report) + + expect(report.request).to eq({ + url: "http://test_host/TEST_PATH?email=[FILTERED]&another_param=thing", + httpMethod: "TEST", + params: { param: 'test' }, + referer: "https://bugsnag.com/about?email=[FILTERED]&another_param=thing", + clientIp: "rack_ip", + headers: { + "Referer" => "https://bugsnag.com/about?email=[FILTERED]&another_param=thing" + } + }) + + expect(report.metadata[:request]).to be(report.request) + expect(report.metadata[:environment]).to eq(rack_env) + expect(report.metadata[:session]).to eq({ session: true }) end it "correctly extracts data from rack middleware" do - callback = double rack_env = { - :env => true, - :HTTP_test_key => "test_key", - "rack.session" => { - :session => true - } + env: true, + HTTP_test_key: "test_key", + "rack.session" => { session: true } } rack_request = double - rack_params = { - :param => 'test' - } allow(rack_request).to receive_messages( - :params => rack_params, - :ip => "rack_ip", - :request_method => "TEST", - :path => "/TEST_PATH", - :scheme => "http", - :host => "test_host", - :port => 80, - :referer => "referer", - :fullpath => "/TEST_PATH" + params: { param: 'test' }, + ip: "rack_ip", + request_method: "TEST", + path: "/TEST_PATH", + scheme: "http", + host: "test_host", + port: 80, + referer: "referer", + fullpath: "/TEST_PATH" ) + expect(Rack::Request).to receive(:new).with(rack_env).and_return(rack_request) - report = double("Bugsnag::Report") - allow(report).to receive(:request_data).and_return({ - :rack_env => rack_env - }) - expect(report).to receive(:context=).with("TEST /TEST_PATH") - expect(report).to receive(:user).and_return({}) - - config = Bugsnag.configuration - config.send_environment = true - config.meta_data_filters = [] - - allow(report).to receive(:configuration).and_return(config) - expect(report).to receive(:add_tab).once.with(:environment, rack_env) - expect(report).to receive(:add_tab).once.with(:request, { - :url => "http://test_host/TEST_PATH", - :httpMethod => "TEST", - :params => rack_params, - :referer => "referer", - :clientIp => "rack_ip", - :headers => { - "Test-Key" => "test_key" - } - }) - expect(report).to receive(:add_tab).once.with(:session, { - :session => true - }) + Bugsnag.configure do |config| + config.send_environment = true + config.meta_data_filters = [] + config.request_data[:rack_env] = rack_env + end + + report = Bugsnag::Report.new(RuntimeError.new('abc'), Bugsnag.configuration) + callback = double expect(callback).to receive(:call).with(report) middleware = Bugsnag::Middleware::RackRequest.new(callback) middleware.call(report) - end - after do - Object.send(:remove_const, :Rack) if @mocked_rack - end + expect(report.request).to eq({ + url: "http://test_host/TEST_PATH", + httpMethod: "TEST", + params: { param: 'test' }, + referer: "referer", + clientIp: "rack_ip", + headers: { "Test-Key" => "test_key" } + }) + expect(report.metadata[:request]).to be(report.request) + expect(report.metadata[:environment]).to eq(rack_env) + expect(report.metadata[:session]).to eq({ session: true }) + end end - it "don't mess with middlewares list on each req" do + it "doesn't change the middleware list on each request" do app = lambda { |env| ['200', {}, ['']] } Bugsnag::Rack.new(app) diff --git a/spec/integrations/rake_spec.rb b/spec/integrations/rake_spec.rb index bfd2cc40b..6139ae86c 100644 --- a/spec/integrations/rake_spec.rb +++ b/spec/integrations/rake_spec.rb @@ -23,8 +23,8 @@ :description => "TEST_COMMENT", :arguments => "TEST_ARGS" }) - expect(report).to receive(:context).with(no_args) - expect(report).to receive(:context=).with("TEST_NAME") + expect(report).to receive(:automatic_context).with(no_args) + expect(report).to receive(:automatic_context=).with("TEST_NAME") expect(callback).to receive(:call).with(report) diff --git a/spec/integrations/resque_spec.rb b/spec/integrations/resque_spec.rb index 952d2337f..c75ac1f75 100644 --- a/spec/integrations/resque_spec.rb +++ b/spec/integrations/resque_spec.rb @@ -85,7 +85,7 @@ def require(path) "class" => "class" } }) - expect(report).to receive(:context=).with(expected_context) + expect(report).to receive(:automatic_context=).with(expected_context) expect(Bugsnag).to receive(:notify).with(exception, true).and_yield(report) resque.save end diff --git a/spec/report_spec.rb b/spec/report_spec.rb index 06edff8bb..46a383f19 100644 --- a/spec/report_spec.rb +++ b/spec/report_spec.rb @@ -2,6 +2,7 @@ require_relative './spec_helper' require 'securerandom' require 'ostruct' +require 'support/shared_examples_for_metadata' module ActiveRecord; class RecordNotFound < RuntimeError; end; end class NestedException < StandardError; attr_accessor :original_exception; end @@ -27,8 +28,281 @@ def gloops end end +shared_examples "Report or Event tests" do |class_to_test| + context "metadata" do + include_examples( + "metadata delegate", + lambda do |metadata, *args| + report = class_to_test.new(RuntimeError.new, Bugsnag.configuration) + report.metadata = metadata + + report.add_metadata(*args) + end, + lambda do |metadata, *args| + report = class_to_test.new(RuntimeError.new, Bugsnag.configuration) + report.metadata = metadata + + report.clear_metadata(*args) + end + ) + end + + it "#headers should return the correct request headers" do + fake_now = Time.gm(2020, 1, 2, 3, 4, 5, 123456) + expect(Time).to receive(:now).twice.and_return(fake_now) + + report_or_event = class_to_test.new( + BugsnagTestException.new("It crashed"), + Bugsnag.configuration + ) + + expect(report_or_event.headers).to eq({ + "Bugsnag-Api-Key" => "c9d60ae4c7e70c4b6c4ebd3e8056d2b8", + "Bugsnag-Payload-Version" => "4.0", + # This matches the time we stubbed earlier (fake_now) + "Bugsnag-Sent-At" => "2020-01-02T03:04:05.123Z" + }) + end + + describe "#summary" do + it "provides a hash of the name, message, and severity" do + begin + 1/0 + rescue ZeroDivisionError => e + report_or_event = class_to_test.new(e, Bugsnag.configuration) + + expect(report_or_event.summary).to eq({ + :error_class => "ZeroDivisionError", + :message => "divided by 0", + :severity => "warning" + }) + end + end + + it "handles strings" do + report_or_event = class_to_test.new("test string", Bugsnag.configuration) + + expect(report_or_event.summary).to eq({ + :error_class => "RuntimeError", + :message => "test string", + :severity => "warning" + }) + end + + it "handles error edge cases" do + report_or_event = class_to_test.new(Timeout::Error, Bugsnag.configuration) + + expect(report_or_event.summary).to eq({ + :error_class => "Timeout::Error", + :message => "Timeout::Error", + :severity => "warning" + }) + end + + it "handles empty exceptions" do + begin + 1/0 + rescue ZeroDivisionError => e + report_or_event = class_to_test.new(e, Bugsnag.configuration) + + report_or_event.exceptions = [] + + expect(report_or_event.summary).to eq({ + :error_class => "Unknown", + :severity => "warning" + }) + end + end + + it "handles removed exceptions" do + begin + 1/0 + rescue ZeroDivisionError => e + report_or_event = class_to_test.new(e, Bugsnag.configuration) + + report_or_event.exceptions = nil + + expect(report_or_event.summary).to eq({ + :error_class => "Unknown", + :severity => "warning" + }) + end + end + + it "handles exceptions being replaced" do + begin + 1/0 + rescue ZeroDivisionError => e + report_or_event = class_to_test.new(e, Bugsnag.configuration) + + report_or_event.exceptions = "no one should ever do this" + + expect(report_or_event.summary).to eq({ + :error_class => "Unknown", + :severity => "warning" + }) + end + end + end + + describe "#errors" do + it "has required attributes" do + exception = RuntimeError.new("example error") + report = class_to_test.new(exception, Bugsnag.configuration) + + expect(report.errors.length).to eq(1) + + error = report.errors.first + + expect(error).to respond_to(:error_class) + expect(error).to respond_to(:error_message) + expect(error).to respond_to(:type) + expect(error).to respond_to(:stacktrace) + + expect(error).to respond_to(:error_class=) + expect(error).to respond_to(:error_message=) + expect(error).to respond_to(:type=) + expect(error).not_to respond_to(:stacktrace=) + end + + it "includes errors that caused the top-most exception" do + begin + begin + raise "one" + rescue + Ruby21Exception.raise!("two") + end + rescue => exception + end + + report = class_to_test.new(exception, Bugsnag.configuration) + + expect(report.errors.length).to eq(2) + + expect(report.errors[0].stacktrace).not_to be_empty + expect(report.errors[0]).to have_attributes({ + error_class: "Ruby21Exception", + error_message: "two", + type: "ruby" + }) + + expect(report.errors[1].stacktrace).not_to be_empty + expect(report.errors[1]).to have_attributes({ + error_class: "RuntimeError", + error_message: "one", + type: "ruby" + }) + end + + it "cannot be assigned to" do + exception = RuntimeError.new("example error") + report = class_to_test.new(exception, Bugsnag.configuration) + + expect(report).not_to respond_to(:errors=) + end + + it "can be mutated" do + exception = RuntimeError.new("example error") + report = class_to_test.new(exception, Bugsnag.configuration) + + report.errors.push("haha") + report.errors.push("haha 2") + report.errors.pop + + expect(report.errors.length).to eq(2) + + expect(report.errors.first.stacktrace).not_to be_empty + expect(report.errors.first).to have_attributes({ + error_class: "RuntimeError", + error_message: "example error", + type: "ruby" + }) + + expect(report.errors[1]).to eq("haha") + end + + it "contains mutable data" do + exception = RuntimeError.new("example error") + report = class_to_test.new(exception, Bugsnag.configuration) + + expect(report.errors.length).to eq(1) + + report.errors.first.error_class = "haha" + report.errors.first.error_message = "ahah" + report.errors.first.type = "aahh" + + expect(report.errors.first.stacktrace).not_to be_empty + expect(report.errors.first).to have_attributes({ + error_class: "haha", + error_message: "ahah", + type: "aahh" + }) + end + + it "shares the stacktrace with #exceptions" do + exception = RuntimeError.new("example error") + report = class_to_test.new(exception, Bugsnag.configuration) + + expect(report.errors.length).to eq(1) + expect(report.exceptions.length).to eq(1) + + error = report.errors.first + exception = report.exceptions.first + + expect(error.stacktrace).not_to be_empty + expect(error.stacktrace).to all(have_key(:lineNumber)) + expect(error.stacktrace).to all(have_key(:file)) + expect(error.stacktrace).to all(have_key(:method)) + expect(error.stacktrace).to all(have_key(:code)) + + expect(error.stacktrace).to be(exception[:stacktrace]) + end + + it "mutating the stacktrace affects the payload" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + expect(report.errors.length).to eq(1) + + error = report.errors.first + + error.stacktrace.clear + error.stacktrace[0] = { + lineNumber: 123, + file: "/dev/null", + method: "do_nothing", + code: "yes, lots" + } + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + exception = get_exception_from_payload(payload) + + expect(exception["stacktrace"]).to eq( + [ + { + "lineNumber" => 123, + "file" => "/dev/null", + "method" => "do_nothing", + "code" => "yes, lots" + } + ] + ) + }) + end + end + + it "has a reference to the original error" do + exception = RuntimeError.new("example error") + report = class_to_test.new(exception, Bugsnag.configuration) + + expect(report.original_error).to be(exception) + end +end + # rubocop:disable Metrics/BlockLength describe Bugsnag::Report do + include_examples("Report or Event tests", Bugsnag::Report) + include_examples("Report or Event tests", Bugsnag::Event) + it "should contain an api_key if one is set" do Bugsnag.notify(BugsnagTestException.new("It crashed")) @@ -92,23 +366,6 @@ def gloops } end - it "#headers should return the correct request headers" do - fake_now = Time.gm(2020, 1, 2, 3, 4, 5, 123456) - expect(Time).to receive(:now).and_return(fake_now) - - report = Bugsnag::Report.new( - BugsnagTestException.new("It crashed"), - Bugsnag.configuration - ) - - expect(report.headers).to eq({ - "Bugsnag-Api-Key" => "c9d60ae4c7e70c4b6c4ebd3e8056d2b8", - "Bugsnag-Payload-Version" => "4.0", - # This matches the time we stubbed earlier (fake_now) - "Bugsnag-Sent-At" => "2020-01-02T03:04:05.123Z" - }) - end - it "has the right exception class" do Bugsnag.notify(BugsnagTestException.new("It crashed")) @@ -189,7 +446,53 @@ def gloops end end - # TODO: nested context + it "metadata added with 'add_metadata' ends up in the payload" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.add_metadata( + :some_tab, + { info: "here", data: "also here" } + ) + + report.add_metadata(:some_other_tab, :info, true) + report.add_metadata(:some_other_tab, :data, "very true") + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["metaData"]).to eq({ + "some_tab" => { + "info" => "here", + "data" => "also here" + }, + "some_other_tab" => { + "info" => true, + "data" => "very true" + } + }) + }) + end + + it "metadata removed with 'clear_metadata' does not end up in the payload" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.add_metadata( + :some_tab, + { info: "here", data: "also here" } + ) + + report.add_metadata(:some_other_tab, :info, true) + report.add_metadata(:some_other_tab, :data, "very true") + + report.clear_metadata(:some_tab) + report.clear_metadata(:some_other_tab, :info) + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["metaData"]).to eq({ + "some_other_tab" => { "data" => "very true" } + }) + }) + end it "accepts tabs in overrides and adds them to metaData" do Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| @@ -199,15 +502,27 @@ def gloops data: "also here" } }) + + report.metadata.merge!({ + some_other_tab: { + info: true, + data: "very true" + } + }) end - expect(Bugsnag).to have_sent_notification{ |payload, headers| + expect(Bugsnag).to(have_sent_notification{ |payload, headers| event = get_event_from_payload(payload) expect(event["metaData"]["some_tab"]).to eq( "info" => "here", "data" => "also here" ) - } + + expect(event["metaData"]["some_other_tab"]).to eq( + "info" => true, + "data" => "very true" + ) + }) end it "accepts meta data from an exception that mixes in Bugsnag::MetaData" do @@ -470,15 +785,89 @@ def gloops } end - it "accepts a user_id in overrides" do + it "uses automatic context if no other context has been set" do Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| - report.user = {id: 'test_user'} + report.automatic_context = "automatic context" end - expect(Bugsnag).to have_sent_notification{ |payload, headers| + expect(Bugsnag).to(have_sent_notification { |payload, _headers| event = get_event_from_payload(payload) - expect(event["user"]["id"]).to eq("test_user") - } + expect(event["context"]).to eq("automatic context") + }) + end + + it "uses Configuration context even if the automatic context has been set" do + Bugsnag.configure do |config| + config.context = "configuration context" + end + + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.automatic_context = "automatic context" + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["context"]).to eq("configuration context") + }) + end + + it "uses overridden context even if the automatic context has been set" do + Bugsnag.configure do |config| + config.context = "configuration context" + end + + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.context = "overridden context" + report.automatic_context = "automatic context" + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["context"]).to eq("overridden context") + }) + end + + it "uses overridden context even it is set to 'nil'" do + Bugsnag.configure do |config| + config.context = nil + end + + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.automatic_context = "automatic context" + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["context"]).to be_nil + }) + end + + it "uses the context from Configuration, if set" do + Bugsnag.configure do |config| + config.context = "example context" + end + + Bugsnag.notify(BugsnagTestException.new("It crashed")) + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["context"]).to eq("example context") + }) + end + + it "allows overriding the context from Configuration" do + Bugsnag.configure do |config| + config.context = "example context" + end + + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.context = "different context" + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["context"]).to eq("different context") + }) end it "does not send an automatic notification if auto_notify is false" do @@ -504,6 +893,26 @@ def gloops } end + it "respects the enabled_release_stages setting by not sending in development" do + Bugsnag.configuration.enabled_release_stages = ["production"] + Bugsnag.configuration.release_stage = "development" + + Bugsnag.notify(BugsnagTestException.new("It crashed")) + + expect(Bugsnag).not_to have_sent_notification + end + + it "respects the enabled_release_stages setting when set" do + Bugsnag.configuration.release_stage = "development" + Bugsnag.configuration.enabled_release_stages = ["development"] + Bugsnag.notify(BugsnagTestException.new("It crashed")) + + expect(Bugsnag).to(have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + expect(event["exceptions"].length).to eq(1) + }) + end + it "respects the notify_release_stages setting by not sending in development" do Bugsnag.configuration.notify_release_stages = ["production"] Bugsnag.configuration.release_stage = "development" @@ -624,7 +1033,8 @@ def gloops :user_secret => "key" } }) - report.meta_data.merge!({ + + report.metadata.merge!({ :session => { :"warden.user.user.key" => "1234", :"warden.user.foobar.key" => "1234", @@ -673,7 +1083,7 @@ def gloops it "filters params from all payload hashes if they are added to meta_data_filters as regex" do Bugsnag.configuration.meta_data_filters << /other_data/ Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| - report.meta_data.merge!({:request => {:params => {:password => "1234", :other_password => "123456", :other_data => "123456"}}}) + report.metadata.merge!({:request => {:params => {:password => "1234", :other_password => "123456", :other_data => "123456"}}}) end expect(Bugsnag).to have_sent_notification{ |payload, headers| @@ -1486,87 +1896,6 @@ def to_s end end - describe "#summary" do - it "provides a hash of the name, message, and severity" do - begin - 1/0 - rescue ZeroDivisionError => e - report = Bugsnag::Report.new(e, Bugsnag.configuration) - - expect(report.summary).to eq({ - :error_class => "ZeroDivisionError", - :message => "divided by 0", - :severity => "warning" - }) - end - end - - it "handles strings" do - report = Bugsnag::Report.new("test string", Bugsnag.configuration) - - expect(report.summary).to eq({ - :error_class => "RuntimeError", - :message => "test string", - :severity => "warning" - }) - end - - it "handles error edge cases" do - report = Bugsnag::Report.new(Timeout::Error, Bugsnag.configuration) - - expect(report.summary).to eq({ - :error_class => "Timeout::Error", - :message => "Timeout::Error", - :severity => "warning" - }) - end - - it "handles empty exceptions" do - begin - 1/0 - rescue ZeroDivisionError => e - report = Bugsnag::Report.new(e, Bugsnag.configuration) - - report.exceptions = [] - - expect(report.summary).to eq({ - :error_class => "Unknown", - :severity => "warning" - }) - end - end - - it "handles removed exceptions" do - begin - 1/0 - rescue ZeroDivisionError => e - report = Bugsnag::Report.new(e, Bugsnag.configuration) - - report.exceptions = nil - - expect(report.summary).to eq({ - :error_class => "Unknown", - :severity => "warning" - }) - end - end - - it "handles exceptions being replaced" do - begin - 1/0 - rescue ZeroDivisionError => e - report = Bugsnag::Report.new(e, Bugsnag.configuration) - - report.exceptions = "no one should ever do this" - - expect(report.summary).to eq({ - :error_class => "Unknown", - :severity => "warning" - }) - end - end - end - if defined?(JRUBY_VERSION) it "works with java.lang.Throwables" do @@ -1586,15 +1915,105 @@ def to_s end it 'includes device data when notify is called' do + fake_device_time = Time.gm(2020, 1, 2, 3, 4, 5, 123456) + fake_sent_at = Time.gm(2021, 1, 2, 3, 4, 5, 123456) + expect(Time).to receive(:now).at_least(:twice).and_return(fake_device_time, fake_sent_at) + Bugsnag.configuration.hostname = 'test-host' Bugsnag.configuration.runtime_versions["ruby"] = '9.9.9' Bugsnag.notify(BugsnagTestException.new("It crashed")) - expect(Bugsnag).to have_sent_notification{ |payload, headers| + expect(Bugsnag).to(have_sent_notification { |payload, headers| event = payload["events"][0] expect(event["device"]["hostname"]).to eq('test-host') expect(event["device"]["runtimeVersions"]["ruby"]).to eq('9.9.9') - } + # This matches the time we stubbed earlier (fake_device_time) + expect(event["device"]["time"]).to eq("2020-01-02T03:04:05.123Z") + + expect(headers["Bugsnag-Api-Key"]).to eq("c9d60ae4c7e70c4b6c4ebd3e8056d2b8") + expect(headers["Bugsnag-Payload-Version"]).to eq("4.0") + # This matches the time we stubbed earlier (fake_sent_at) + expect(headers["Bugsnag-Sent-At"]).to eq("2021-01-02T03:04:05.123Z") + }) + end + + context "#user" do + it "accepts an arbitrary user hash" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.user = { id: "test_user", abc: "xyz" } + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["user"]["id"]).to eq("test_user") + expect(event["user"]["abc"]).to eq("xyz") + }) + end + + it "set_user will set the three supported fields" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.set_user("123", "abc.xyz@example.com", "abc xyz") + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["user"]["id"]).to eq("123") + expect(event["user"]["email"]).to eq("abc.xyz@example.com") + expect(event["user"]["name"]).to eq("abc xyz") + }) + end + + it "set_user will not set fields that are 'nil'" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.set_user("123", nil, "abc xyz") + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["user"]["id"]).to eq("123") + expect(event["user"]).not_to have_key("email") + expect(event["user"]["name"]).to eq("abc xyz") + }) + end + + it "set_user will unset all fields if passed no parameters" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.user = { id: "nope", email: "nah@example.com", name: "yes", other: "stuff" } + + report.set_user + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["user"]).to be_empty + }) + end + + it "set_user can be passed only an ID" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.set_user("123") + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["user"]["id"]).to eq("123") + expect(event["user"]).not_to have_key("email") + expect(event["user"]).not_to have_key("name") + }) + end + + it "set_user can be passed only an ID and email" do + Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report| + report.set_user("123", "123@example.com") + end + + expect(Bugsnag).to(have_sent_notification { |payload, _headers| + event = get_event_from_payload(payload) + expect(event["user"]["id"]).to eq("123") + expect(event["user"]["email"]).to eq("123@example.com") + expect(event["user"]).not_to have_key("name") + }) + end end end # rubocop:enable Metrics/BlockLength diff --git a/spec/session_tracker_spec.rb b/spec/session_tracker_spec.rb index e208494c8..291262081 100644 --- a/spec/session_tracker_spec.rb +++ b/spec/session_tracker_spec.rb @@ -16,7 +16,7 @@ res.status = 202 res.body = "OK\n" end - Thread.new{ server.start } + Thread.new { server.start } end before(:each) do @@ -78,6 +78,26 @@ expect(Bugsnag.session_tracker.session_counts.size).to eq(0) end + it 'will not create sessions if the release stage is not enabled' do + Bugsnag.configure do |config| + config.enabled_release_stages = ['abc'] + config.release_stage = 'xyz' + end + + expect(Bugsnag.configuration.enable_sessions).to eq(true) + expect(Bugsnag.session_tracker.session_counts.size).to eq(0) + + Bugsnag.start_session + + expect(Bugsnag.session_tracker.session_counts.size).to eq(0) + + Bugsnag.configuration.release_stage = 'abc' + + Bugsnag.start_session + + expect(Bugsnag.session_tracker.session_counts.size).to eq(1) + end + it 'sends sessions when send_sessions is called' do Bugsnag.configure do |conf| conf.auto_capture_sessions = true diff --git a/spec/support/shared_examples_for_metadata.rb b/spec/support/shared_examples_for_metadata.rb new file mode 100644 index 000000000..eb869abe7 --- /dev/null +++ b/spec/support/shared_examples_for_metadata.rb @@ -0,0 +1,157 @@ +require 'spec_helper' + +# shared examples for #add_metadata and #clear_metadata +# use by providing two functions - one that implements add_metadata and one +# that implements clear_metadata, e.g. +# +# RSpec.describe SomeClass do +# include_examples( +# 'metadata delegate', +# ->(metadata, *args) { SomeClass.new(metadata).add_metadata(*args) }, +# ->(metadata, *args) { SomeClass.new(metadata).clear_metadata(*args) } +# ) +# end +RSpec.shared_examples 'metadata delegate' do |add_metadata, clear_metadata| + context "#add_metadata" do + context "with 'section', 'key' and 'value'" do + it "adds the given key/value pair to the section" do + metadata = {} + + add_metadata.call(metadata, :abc, :xyz, "Hello!") + + expect(metadata).to eq({ abc: { xyz: "Hello!" } }) + end + + it "merges the new key/value pair with existing data in the section" do + metadata = { abc: { a: 1, b: 2, c: 3 } } + + add_metadata.call(metadata, :abc, :xyz, "Hello!") + + expect(metadata).to eq({ abc: { a: 1, b: 2, c: 3, xyz: "Hello!" } }) + end + + it "replaces existing metadata if the 'key' already exists" do + metadata = { abc: { xyz: "Hello!" } } + + add_metadata.call(metadata, :abc, :xyz, "Goodbye!") + + expect(metadata).to eq({ abc: { xyz: "Goodbye!" } }) + end + + it "removes the key if 'value' is nil" do + metadata = { abc: { xyz: "Hello!" } } + + add_metadata.call(metadata, :abc, :xyz, nil) + + expect(metadata).to eq({ abc: {} }) + end + end + + context "with 'section' and 'data'" do + it "adds the data to the given section" do + metadata = {} + + add_metadata.call(metadata, :xyz, { abc: "Hello!" }) + + expect(metadata).to eq({ xyz: { abc: "Hello!" } }) + end + + it "merges the new data with any existing data in the section" do + metadata = { xyz: { x: 1, y: 2, z: 3 } } + + add_metadata.call(metadata, :xyz, { abc: "Hello!" }) + + expect(metadata).to eq({ xyz: { x: 1, y: 2, z: 3, abc: "Hello!" } }) + end + + it "does not deep merge conflicting data in the section" do + metadata = { xyz: { x: { a: 1, b: 2 } } } + + add_metadata.call(metadata, :xyz, { x: { c: 3 } }) + + # if this was a deep merge, metadata[:xyz][:x] would be { a: 1, b: 2, c: 3 } + expect(metadata).to eq({ xyz: { x: { c: 3 } } }) + end + + it "replaces existing keys in the section" do + metadata = { xyz: { x: 0, z: "yes", abc: "??" } } + + add_metadata.call(metadata, :xyz, { x: 1, y: 2, z: 3 }) + + expect(metadata).to eq({ xyz: { x: 1, y: 2, z: 3, abc: "??" } }) + end + + it "removes keys that have a value of 'nil'" do + metadata = { xyz: { x: 0, z: "yes", abc: "??" } } + + add_metadata.call(metadata, :xyz, { x: nil, y: 2, z: 3 }) + + expect(metadata).to eq({ xyz: { y: 2, z: 3, abc: "??" } }) + end + end + + context "with bad parameters" do + it "does nothing if called with a 'key' but no 'value'" do + metadata = {} + + add_metadata.call(metadata, :a, :b) + + expect(metadata).to be_empty + end + + it "does nothing if called with a Hash 'key' and a value" do + metadata = {} + + add_metadata.call(metadata, :a, { b: 1 }, 123) + + expect(metadata).to be_empty + end + end + end + + context "#clear_metadata" do + context "with 'section'" do + it "removes the given section from metadata" do + metadata = { some: "data", goes: "here" } + + clear_metadata.call(metadata, :some) + + expect(metadata).to eq({ goes: "here" }) + end + + it "does nothing if the section does not exist" do + metadata = { some: "data", goes: "here" } + + clear_metadata.call(metadata, :does_not_exist) + + expect(metadata).to eq({ some: "data", goes: "here" }) + end + end + + context "with 'section' and 'key'" do + it "removes the given 'key' from 'section'" do + metadata = { some: { data: { goes: "here" }, also: "there" } } + + clear_metadata.call(metadata, :some, :data) + + expect(metadata).to eq({ some: { also: "there" } }) + end + + it "does nothing if the section does not exist" do + metadata = { some: { data: { goes: "here" }, also: "there" } } + + clear_metadata.call(metadata, :nope, :data) + + expect(metadata).to eq({ some: { data: { goes: "here" }, also: "there" } }) + end + + it "does nothing if the key does not exist in the section" do + metadata = { some: { data: { goes: "here" }, also: "there" } } + + clear_metadata.call(metadata, :some, :nah) + + expect(metadata).to eq({ some: { data: { goes: "here" }, also: "there" } }) + end + end + end +end diff --git a/spec/utility/metadata_delegate_spec.rb b/spec/utility/metadata_delegate_spec.rb new file mode 100644 index 000000000..f08edcdff --- /dev/null +++ b/spec/utility/metadata_delegate_spec.rb @@ -0,0 +1,11 @@ +require 'spec_helper' +require 'bugsnag/utility/metadata_delegate' +require 'support/shared_examples_for_metadata' + +RSpec.describe Bugsnag::Utility::MetadataDelegate do + include_examples( + 'metadata delegate', + Bugsnag::Utility::MetadataDelegate.new.method(:add_metadata), + Bugsnag::Utility::MetadataDelegate.new.method(:clear_metadata) + ) +end