get sentry into canvas

closes CNVS-6016

No more error reports!  (soon)

this commit builds up sentry integration through the new
Canvas::Errors module, along with other things that need
to happen on every exception.  ErrorReports
should now get pushed towards just being used for representing
a complaint a user filed via the get help form.

I fixed about half the things that got linted as well
while I was in here, but because this touches to much
I fear divergence from tackling too many (I think we
can safely say it's "better than we found it")

I left a lot of the infrastructure for error reports in place
until other commits for plugins can be merged

TEST PLAN:
 1) setup your raven.yml config file with the dsn for our
  sentry install
 2) force an error to happen in a request response cycle.
 3) see the error in sentry
 4) force an error to happen in a job
 5) see the error in sentry
 6) statsd increments shoudl still fire
 7) for the moment, an error report should still get created.

Change-Id: I5a9dc7214598f8d5083451fd15f0423f8f939034
Reviewed-on: https://gerrit.instructure.com/51621
Reviewed-by: Simon Williams <simon@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
Tested-by: Jenkins
QA-Review: August Thornton <august@instructure.com>
Product-Review: Ethan Vizitei <evizitei@instructure.com>

Gemfile.d/other_stuff.rb

@@ -35,7 +35,7 @@ gem 'bcrypt-ruby', '3.0.1'
 gem 'canvas_connect', '0.3.7'
   gem 'adobe_connect', '1.0.2', require: false
 gem 'canvas_webex', '0.15'
-gem 'canvas-jobs', '0.9.11'
+gem 'canvas-jobs', '0.9.12'
 
 gem 'ffi', '1.1.5', require: false
 gem 'hairtrigger', '0.2.12'
@@ -97,6 +97,7 @@ gem 'foreigner', '0.9.2'
 gem 'crocodoc-ruby', '0.0.1', require: false
 gem 'hey', '1.3.0', require: false
 gem 'aroi', '0.0.2'
+gem 'sentry-raven', '0.12.3', require: false
 
 gem 'active_polymorph', path: 'gems/active_polymorph'
 gem 'activesupport-suspend_callbacks', path: 'gems/activesupport-suspend_callbacks'

app/controllers/application_controller.rb

@@ -932,7 +932,7 @@ class ApplicationController < ActionController::Base
       logger.fatal("#{message}\n\n")
     end
 
-    if config.consider_all_requests_local || local_request?
+    if config.consider_all_requests_local
       rescue_action_locally(exception)
     else
       rescue_action_in_public(exception)
@@ -971,23 +971,11 @@ class ApplicationController < ActionController::Base
       type = '404' if status == '404 Not Found'
 
       unless exception.respond_to?(:skip_error_report?) && exception.skip_error_report?
-        error_info = {
+        opts = {type: type}
-          :url => request.url,
+        info = Canvas::Errors::Info.new(request, @domain_root_account, @current_user, opts)
-          :user => @current_user,
+        error_info = info.to_h
-          :user_agent => request.headers['User-Agent'],
+        capture_outputs = Canvas::Errors.capture(exception, error_info)
-          :request_context_id => RequestContextGenerator.request_id,
+        error = ErrorReport.find(capture_outputs[:error_report])
-          :account => @domain_root_account,
-          :request_method => request.request_method_symbol,
-          :format => request.format,
-        }.merge(ErrorReport.useful_http_env_stuff_from_request(request))
-
-        error = if @domain_root_account
-          @domain_root_account.shard.activate do
-            ErrorReport.log_exception(type, exception, error_info)
-          end
-        else
-          ErrorReport.log_exception(type, exception, error_info)
-        end
       end
 
       if api_request?
@@ -997,8 +985,8 @@ class ApplicationController < ActionController::Base
       end
     rescue => e
       # error generating the error page? failsafe.
+      Canvas::Errors.capture(e)
       render_optional_error_file response_code_for_rescue(exception)
-      ErrorReport.log_exception(:default, e)
     end
   end
 
@@ -1096,10 +1084,6 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  def local_request?
-    false
-  end
-
   def claim_session_course(course, user, state=nil)
     e = course.claim_with_teacher(user)
     session[:claimed_enrollment_uuids] ||= []

app/controllers/assignments_controller.rb

@@ -173,12 +173,12 @@ class AssignmentsController < ApplicationController
       docs = {}
       begin
         docs = google_service_connection.list_with_extension_filter(assignment.allowed_extensions)
-      rescue GoogleDocs::NoTokenError
+      rescue GoogleDocs::NoTokenError => e
-        #do nothing
+        CanvasErrors.capture_exception(:oauth, e)
-      rescue ArgumentError
+      rescue ArgumentError => e
-        #do nothing
+        CanvasErrors.capture_exception(:oauth, e)
       rescue => e
-        ErrorReport.log_exception(:oauth, e)
+        CanvasErrors.capture_exception(:oauth, e)
         raise e
       end
       respond_to do |format|

app/controllers/context_controller.rb

@@ -290,13 +290,14 @@ class ContextController < ApplicationController
   def roster_user
     if authorized_action(@context, @current_user, :read_roster)
       if params[:id] !~ Api::ID_REGEX
-        # todo stop generating an error report and fix the bad input
+        # TODO: stop generating an error report and fix the bad input
-        ErrorReport.log_error('invalid_user_id',
+
-                              {message: "invalid user_id in ContextController::roster_user",
+        env_stuff = Canvas::Errors::Info.useful_http_env_stuff_from_request(request)
-                               current_user_id: @current_user.id,
+        Canvas::Errors.capture('invalid_user_id', {
-                               current_user_name: @current_user.sortable_name}.
+          message: "invalid user_id in ContextController::roster_user",
-                                merge(ErrorReport.useful_http_env_stuff_from_request(request))
+          current_user_id: @current_user.id,
-        )
+          current_user_name: @current_user.sortable_name
+        }.merge(env_stuff))
         raise ActiveRecord::RecordNotFound
       end
       user_id = Shard.relative_id_for(params[:id], Shard.current, @context.shard)

app/controllers/info_controller.rb

@@ -52,19 +52,23 @@ class InfoController < ApplicationController
       @report.account ||= @domain_root_account
       backtrace = params[:error].delete(:backtrace) rescue nil
       backtrace ||= ""
-      backtrace += "\n\n-----------------------------------------\n\n" + @report.backtrace if @report.backtrace
+      if @report.backtrace
+        backtrace += "\n\n-----------------------------------------\n\n"
+        backtrace += @report.backtrace
+      end
       @report.backtrace = backtrace
-      @report.http_env ||= ErrorReport.useful_http_env_stuff_from_request(request)
+      @report.http_env ||= Canvas::Errors::Info.useful_http_env_stuff_from_request(request)
       @report.request_context_id = RequestContextGenerator.request_id
       @report.assign_data(error)
       @report.save
       @report.send_later(:send_to_external)
     rescue => e
       @exception = e
-      ErrorReport.log_exception(:default, e,
+      Canvas::Errors.capture(
-        :message => "Error Report Creation failed",
+        e,
-        :user_email => (error[:email] rescue ''),
+        message: "Error Report Creation failed",
-        :user_id => @current_user.try(:id)
+        user_email: error[:email],
+        user_id: @current_user.try(:id)
       )
     end
     respond_to do |format|

app/controllers/submissions_controller.rb

@@ -597,7 +597,7 @@ class SubmissionsController < ApplicationController
       begin
         @submissions = @assignment.update_submission(@user, params[:submission])
       rescue => e
-        ErrorReport.log_exception(:submissions, e)
+        Canvas::Errors.capture_exception(:submissions, e)
         logger.error(e)
       end
       respond_to do |format|

app/controllers/users_controller.rb

@@ -238,9 +238,9 @@ class UsersController < ApplicationController
         end
 
         flash[:notice] = t('google_drive_added', "Google Drive account successfully added!")
-        redirect_to json['return_to_url'] and return
+        return redirect_to(json['return_to_url'])
       rescue => e
-        ErrorReport.log_exception(:oauth, e)
+        Canvas::Errors.capture_exception(:oauth, e)
         flash[:error] = t('google_drive_fail', "Google Drive authorization failed. Please try again")
       end
     end
@@ -274,7 +274,7 @@ class UsersController < ApplicationController
 
           flash[:notice] = t('google_docs_added', "Google Docs access authorized!")
         rescue => e
-          ErrorReport.log_exception(:oauth, e)
+          Canvas::Errors.capture_exception(:oauth, e)
           flash[:error] = t('google_docs_fail', "Google Docs authorization failed. Please try again")
         end
       elsif params[:service] == "linked_in"
@@ -302,7 +302,7 @@ class UsersController < ApplicationController
 
           flash[:notice] = t('linkedin_added', "LinkedIn account successfully added!")
         rescue => e
-          ErrorReport.log_exception(:oauth, e)
+          Canvas::Errors.capture_exception(:oauth, e)
           flash[:error] = t('linkedin_fail', "LinkedIn authorization failed. Please try again")
         end
       else
@@ -327,7 +327,7 @@ class UsersController < ApplicationController
 
           flash[:notice] = t('twitter_added', "Twitter access authorized!")
         rescue => e
-          ErrorReport.log_exception(:oauth, e)
+          Canvas::Errors.capture_exception(:oauth, e)
           flash[:error] = t('twitter_fail_whale', "Twitter authorization failed. Please try again")
         end
       end

app/helpers/attachment_helper.rb

@@ -23,7 +23,7 @@ module AttachmentHelper
       begin
         attrs[:crocodoc_session_url] = attachment.crocodoc_url(@current_user)
       rescue => e
-        ErrorReport.log_exception('crocodoc', e)
+        Canvas::Errors.capture_exception(:crocodoc, e)
       end
     elsif attachment.canvadocable?
       attrs[:canvadoc_session_url] = attachment.canvadoc_url(@current_user)

app/models/account_authorization_config.rb

@@ -385,17 +385,16 @@ class AccountAuthorizationConfig < ActiveRecord::Base
 
     default_timeout = Setting.get('ldap_timelimit', 5.seconds.to_s).to_f
 
-    Canvas.timeout_protection("ldap:#{self.global_id}",
+    timeout_options = { raise_on_timeout: true, fallback_timeout_length: default_timeout }
-                              raise_on_timeout: true,
+    Canvas.timeout_protection("ldap:#{self.global_id}", timeout_options) do
-                              fallback_timeout_length: default_timeout) do
+      ldap = self.ldap_connection
-                                ldap = self.ldap_connection
+      filter = self.ldap_filter(unique_id)
-                                filter = self.ldap_filter(unique_id)
+      ldap.bind_as(base: ldap.base, filter: filter, password: password_plaintext)
-                                ldap.bind_as(:base => ldap.base, :filter => filter, :password => password_plaintext)
+    end
-                              end
   rescue => e
-    ErrorReport.log_exception(:ldap, e, :account => self.account)
+    Canvas::Errors.capture(e, type: :ldap, account: self.account)
     if e.is_a?(Timeout::Error)
-      self.update_attribute(:last_timeout_failure, Time.now)
+      self.update_attribute(:last_timeout_failure, Time.zone.now)
     end
     return nil
   end

app/models/assessment_question.rb

@@ -107,8 +107,10 @@ class AssessmentQuestion < ActiveRecord::Base
               new_file = file.clone_for(self)
             rescue => e
               new_file = nil
-              er = ErrorReport.log_exception(:file_clone_during_translate_links, e)
+              er_id = Canvas::Errors.capture_exception(:file_clone_during_translate_links, e)[:error_report]
-              logger.error("Error while cloning attachment during AssessmentQuestion#translate_links: id: #{self.id} error_report: #{er.id}")
+              logger.error("Error while cloning attachment during"\
+                           " AssessmentQuestion#translate_links: "\
+                           "id: #{self.id} error_report: #{er_id}")
             end
             new_file.save if new_file
             file_substitutions[id_or_path] = new_file

app/models/attachment.rb

@@ -1199,7 +1199,7 @@ class Attachment < ActiveRecord::Base
     end
   rescue => e
     update_attribute(:workflow_state, 'errored')
-    ErrorReport.log_exception(:canvadocs, e, :attachment_id => id)
+    Canvas::Errors.capture(e, type: :canvadocs, attachment_id: id)
 
     if attempt <= Setting.get('max_canvadocs_attempts', '5').to_i
       send_later_enqueue_args :submit_to_canvadocs, {
@@ -1219,7 +1219,7 @@ class Attachment < ActiveRecord::Base
     end
   rescue => e
     update_attribute(:workflow_state, 'errored')
-    ErrorReport.log_exception(:crocodoc, e, :attachment_id => id)
+    Canvas::Errors.capture(e, type: :canvadocs, attachment_id: id)
 
     if attempt <= Setting.get('max_crocodoc_attempts', '5').to_i
       send_later_enqueue_args :submit_to_crocodoc, {

app/models/content_export.rb

@@ -269,16 +269,17 @@ class ContentExport < ActiveRecord::Base
     self.settings[:errors] ||= []
     er = nil
     if exception_or_info.is_a?(Exception)
-      er = ErrorReport.log_exception(:course_export, exception_or_info)
+      out = Canvas::Errors.capture_exception(:course_export, exception_or_info)
-      self.settings[:errors] << [user_message, "ErrorReport id: #{er.id}"]
+      er = out[:error_report]
+      self.settings[:errors] << [user_message, "ErrorReport id: #{er}"]
     else
       self.settings[:errors] << [user_message, exception_or_info]
     end
     if self.content_migration
-      self.content_migration.add_issue(user_message, :error, :error_report_id => er && er.id)
+      self.content_migration.add_issue(user_message, :error, error_report_id: er)
     end
   end
-  
+
   def root_account
     self.context.try_rescue(:root_account)
   end

app/models/content_migration.rb

@@ -198,8 +198,8 @@ class ContentMigration < ActiveRecord::Base
     if opts[:error_report_id]
       mi.error_report_id = opts[:error_report_id]
     elsif opts[:exception]
-      er = ErrorReport.log_exception(:content_migration, opts[:exception])
+      er = Canvas::Errors.capture_exception(:content_migration, opts[:exception])[:error_report]
-      mi.error_report_id = er.id
+      mi.error_report_id = er
     end
     mi.error_message = opts[:error_message]
     mi.fix_issue_html_url = opts[:fix_issue_html_url]
@@ -327,7 +327,7 @@ class ContentMigration < ActiveRecord::Base
           self.workflow_state = 'failed'
           message = "The migration plugin #{migration_type} doesn't have a worker."
           migration_settings[:last_error] = message
-          ErrorReport.log_exception(:content_migration, $!)
+          Canvas::Errors.capture_exception(:content_migration, $ERROR_INFO)
           logger.error message
           self.save
         end
@@ -444,8 +444,8 @@ class ContentMigration < ActiveRecord::Base
       end
     rescue => e
       self.workflow_state = :failed
-      er = ErrorReport.log_exception(:content_migration, e)
+      er_id = Canvas::Errors.capture_exception(:content_migration, e)[:error_report]
-      migration_settings[:last_error] = "ErrorReport:#{er.id}"
+      migration_settings[:last_error] = "ErrorReport:#{er_id}"
       logger.error e
       self.save
       raise e

app/models/crocodoc_document.rb

@@ -146,7 +146,7 @@ class CrocodocDocument < ActiveRecord::Base
             if status['status'] == 'ERROR'
               error = status['error'] || 'No explanation given'
               error_uuids << status['uuid']
-              ErrorReport.log_error 'crocodoc', :message => error
+              Canvas::Errors.capture 'crocodoc', message: error
             end
           end
 

app/models/delayed_notification.rb

@@ -60,9 +60,7 @@ class DelayedNotification < ActiveRecord::Base
     self.do_process unless self.new_record?
     res
   rescue => e
-    ErrorReport.log_exception(:default, e, {
+    Canvas::Errors.capture(e, message: "Delayed Notification processing failed")
-      :message => "Delayed Notification processing failed",
-    })
     logger.error "delayed notification processing failed: #{e.message}\n#{e.backtrace.join "\n"}"
     self.workflow_state = 'errored'
     self.save

app/models/error_report.rb

@@ -53,11 +53,11 @@ class ErrorReport < ActiveRecord::Base
       @opts = opts
       # sanitize invalid encodings
       @opts[:message] = Utf8Cleaner.strip_invalid_utf8(@opts[:message]) if @opts[:message]
-      @opts[:exception_message] = Utf8Cleaner.strip_invalid_utf8(@opts[:exception_message]) if @opts[:exception_message]
+      if @opts[:exception_message]
+        @opts[:exception_message] = Utf8Cleaner.strip_invalid_utf8(@opts[:exception_message])
+      end
       @opts[:hostname] = self.class.hostname
       @opts[:pid] = Process.pid
-      CanvasStatsd::Statsd.increment("errors.all")
-      CanvasStatsd::Statsd.increment("errors.#{category}")
       run_callbacks :on_log_error
       create_error_report(opts)
     end
@@ -101,6 +101,32 @@ class ErrorReport < ActiveRecord::Base
     Reporter.new.log_exception(category, exception, opts)
   end
 
+  def self.log_captured(type, exception, error_report_info)
+    if exception.is_a?(String) || exception.is_a?(Symbol)
+      log_error(exception, error_report_info)
+    else
+      type = exception.class.name if type == :default
+      log_exception(type, exception, error_report_info)
+    end
+  end
+
+  def self.log_exception_from_canvas_errors(exception, data)
+    tags = data.fetch(:tags, {})
+    account_id = tags[:account_id]
+    domain_root_account = account_id ? Account.where(id: account_id).first : nil
+    error_report_info = tags.merge(data[:extra])
+    type = tags.fetch(:type, :default)
+
+    if domain_root_account
+      domain_root_account.shard.activate do
+        ErrorReport.log_captured(type, exception, error_report_info)
+      end
+    else
+      ErrorReport.log_captured(type, exception, error_report_info)
+    end
+  end
+
+
   # assigns data attributes to the column if there's a column with that name,
   # otherwise goes into the general data hash
   def assign_data(data = {})
@@ -153,33 +179,6 @@ class ErrorReport < ActiveRecord::Base
     self.where("created_at<?", before_date).delete_all
   end
 
-  USEFUL_ENV = [
-    "HTTP_ACCEPT",
-    "HTTP_ACCEPT_ENCODING",
-    "HTTP_HOST",
-    "HTTP_REFERER",
-    "HTTP_USER_AGENT",
-    "PATH_INFO",
-    "QUERY_STRING",
-    "REMOTE_HOST",
-    "REQUEST_METHOD",
-    "REQUEST_PATH",
-    "REQUEST_URI",
-    "SERVER_NAME",
-    "SERVER_PORT",
-    "SERVER_PROTOCOL",
-  ]
-  def self.useful_http_env_stuff_from_request(request)
-    stuff = request.env.slice(*USEFUL_ENV)
-    stuff['REMOTE_ADDR'] = request.remote_ip # ActionDispatch::Request#remote_ip has proxy smarts
-    stuff['QUERY_STRING'] = LoggingFilter.filter_query_string("?" + stuff['QUERY_STRING'])
-    stuff['REQUEST_URI'] = LoggingFilter.filter_uri(request.url)
-    stuff['path_parameters'] = LoggingFilter.filter_params(request.path_parameters.dup).inspect # params rails picks up from the url
-    stuff['query_parameters'] = LoggingFilter.filter_params(request.query_parameters.dup).inspect # params rails picks up from the query string
-    stuff['request_parameters'] = LoggingFilter.filter_params(request.request_parameters.dup).inspect # params from forms
-    Marshal.load(Marshal.dump(stuff))
-  end
-
   def self.categories
     distinct('category')
   end

app/models/importers/course_content_importer.rb

@@ -92,8 +92,9 @@ module Importers
           begin
             self.import_media_objects(mo_attachments, migration)
           rescue => e
-            er = ErrorReport.log_exception(:import_media_objects, e)
+            er = Canvas::Errors.capture_exception(:import_media_objects, e)[:error_report]
-            migration.add_error(t(:failed_import_media_objects, %{Failed to import media objects}), error_report_id: er.id)
+            error_message = t('Failed to import media objects')
+            migration.add_error(error_message, error_report_id: er)
           end
         end
         if migration.canvas_import?

app/models/importers/wiki_page_importer.rb

@@ -19,11 +19,12 @@ module Importers
     def self.process_migration(data, migration)
       wikis = data['wikis'] ? data['wikis']: []
       wikis.each do |wiki|
-        if !wiki
+        unless wiki
-          ErrorReport.log_error(:content_migration, :message => "There was a nil wiki page imported for ContentMigration:#{migration.id}")
+          message = "There was a nil wiki page imported for ContentMigration:#{migration.id}"
+          Canvas::Errors.capture(:content_migration, message: message)
           next
         end
-        next unless migration.import_object?("wiki_pages", wiki['migration_id']) || migration.import_object?("wikis", wiki['migration_id'])
+        next unless wiki_page_migration?(migration, wiki)
         begin
           self.import_from_migration(wiki, migration.context, migration) if wiki
         rescue
@@ -32,6 +33,12 @@ module Importers
       end
     end
 
+    def self.wiki_page_migration?(migration, wiki)
+      migration.import_object?("wiki_pages", wiki['migration_id']) ||
+        migration.import_object?("wikis", wiki['migration_id'])
+    end
+    private_class_method :wiki_page_migration?
+
     def self.import_from_migration(hash, context, migration=nil, item=nil)
       hash = hash.with_indifferent_access
       item ||= WikiPage.where(wiki_id: context.wiki, id: hash[:id]).first

app/models/media_object.rb

@@ -197,12 +197,12 @@ class MediaObject < ActiveRecord::Base
   def retrieve_details_ensure_codecs(attempt=0)
     retrieve_details
     if (!self.data || !self.data[:extensions] || !self.data[:extensions][:flv]) && self.created_at > 6.hours.ago
-      if(attempt < 10)
+      if attempt < 10
         send_at((5 * attempt).minutes.from_now, :retrieve_details_ensure_codecs, attempt + 1)
       else
-        ErrorReport.log_error(:default, {
+        Canvas::Errors.capture(:media_object_failure, {
-          :message => "Kaltura flavor retrieval failed",
+          message: "Kaltura flavor retrieval failed",
-          :object => self.inspect.to_s,
+          object: self.inspect.to_s,
         })
       end
     end

app/models/message.rb

@@ -694,10 +694,12 @@ class Message < ActiveRecord::Base
       raise_error = @exception.to_s !~ /^450/
       log_error = raise_error && !@exception.is_a?(Timeout::Error)
       if log_error
-        ErrorReport.log_exception(:default, @exception, {
+        Canvas::Errors.capture(
-          :message => 'Message delivery failed',
+          @exception,
-          :to => to,
+          message: 'Message delivery failed',
-          :object => inspect.to_s })
+          to: to,
+          object: inspect.to_s
+        )
       end
 
       self.errored_dispatch

app/models/progress.rb

@@ -97,8 +97,8 @@ class Progress < ActiveRecord::Base
     end
 
     def on_permanent_failure(error)
-      error_report = ErrorReport.log_exception("Progress::Work", error)
+      er_id = Canvas::Errors.capture_exception("Progress::Work", error)[:error_report]
-      @progress.message = "Unexpected error, ID: #{error_report.id rescue "unknown"}"
+      @progress.message = "Unexpected error, ID: #{er_id || 'unknown'}"
       @progress.save
       @progress.fail
     end

app/models/pseudonym.rb

@@ -444,10 +444,11 @@ class Pseudonym < ActiveRecord::Base
     end
     !!res
   rescue => e
-    ErrorReport.log_exception(:ldap, e, {
+    Canvas::Errors.capture(e, {
-      :message => "LDAP authentication error",
+      type: :ldap,
-      :object => self.inspect.to_s,
+      message: "LDAP authentication error",
-      :unique_id => self.unique_id,
+      object: self.inspect.to_s,
+      unique_id: self.unique_id,
     })
     nil
   end

app/models/zip_file_import.rb

@@ -75,10 +75,10 @@ class ZipFileImport < ActiveRecord::Base
     self.workflow_state = :imported
     self.save
   rescue => e
-    ErrorReport.log_exception(:zip_file_import, e)
+    Canvas::Errors.capture_exception(:zip_file_import, e)
 
     self.data[:error_message] = e.to_s
-    self.data[:stack_trace] = "#{e.to_s}\n#{e.backtrace.join("\n")}"
+    self.data[:stack_trace] = "#{e}\n#{e.backtrace.join("\n")}"
     self.workflow_state = "failed"
     self.save
   end

config/initializers/active_record.rb

@@ -206,10 +206,10 @@ class ActiveRecord::Base
   def touch_context
     return if (@@skip_touch_context ||= false || @skip_touch_context ||= false)
     if self.respond_to?(:context_type) && self.respond_to?(:context_id) && self.context_type && self.context_id
-      self.context_type.constantize.update_all({ :updated_at => Time.now.utc }, { :id => self.context_id })
+      self.context_type.constantize.update_all({ updated_at: Time.now.utc }, { id: self.context_id })
     end
   rescue
-    ErrorReport.log_exception(:touch_context, $!)
+    Canvas::Errors.capture_exception(:touch_context, $ERROR_INFO)
   end
 
   def touch_user
@@ -225,7 +225,7 @@ class ActiveRecord::Base
     end
     true
   rescue
-    ErrorReport.log_exception(:touch_user, $!)
+    Canvas::Errors.capture_exception(:touch_user, $ERROR_INFO)
     false
   end
 

config/initializers/delayed_job.rb

@@ -101,5 +101,11 @@ Delayed::Worker.lifecycle.before(:perform) do |job|
 end
 
 Delayed::Worker.lifecycle.before(:exceptional_exit) do |worker, exception|
-  ErrorReport.log_exception(:delayed_jobs, exception) rescue nil
+  info = Canvas::Errors::JobInfo.new(nil, worker)
+  Canvas::Errors.capture(exception, info.to_h)
+end
+
+Delayed::Worker.lifecycle.before(:error) do |worker, job, exception|
+  info = Canvas::Errors::JobInfo.new(job, worker)
+  Canvas::Errors.capture(exception, info.to_h)
 end

config/initializers/errors.rb

@@ -0,0 +1,21 @@
+# This initializer registers the two interal tools canvas uses
+# for tracking errors.  One (:error_report) creates a record in the
+# error_reports database table for each exception that occurs.  The
+# other (:error_stats) will send two counter increments to statsd,
+# one for "all" which tabulates every error that occurs, and one
+# that includes the class of the exception in the key so you
+# can see big spikes in a certain kind of error.  Either can be
+# disabled individually with a setting.
+#
+Canvas::Errors.register!(:error_report) do |exception, data|
+  setting = Setting.get("error_report_exception_handling", 'true')
+  if setting == 'true'
+    report = ErrorReport.log_exception_from_canvas_errors(exception, data)
+    report.try(:global_id)
+  end
+end
+
+Canvas::Errors.register!(:error_stats) do |exception, data|
+  setting = Setting.get("collect_error_statistics", 'true')
+  Canvas::ErrorStats.capture(exception, data) if setting == 'true'
+end

config/initializers/periodic_jobs.rb

@@ -15,7 +15,8 @@ Rails.configuration.after_initialize do
     expire_after ||= 1.day
 
     Delayed::Periodic.cron 'ActiveRecord::SessionStore::Session.delete_all', '*/5 * * * *' do
-      Shard.with_each_shard(exception: -> { ErrorReport.log_exception(:periodic_job, $!) }) do
+      callback = -> { Canvas::Errors.capture_exception(:periodic_job, $ERROR_INFO) }
+      Shard.with_each_shard(exception: callback) do
         ActiveRecord::SessionStore::Session.delete_all(['updated_at < ?', expire_after.ago])
       end
     end

config/initializers/sentry.rb

@@ -0,0 +1,26 @@
+# This initializer is for the Sentry exception tracking system.
+#
+# "Raven" is the ruby library that is the client to sentry, and it's
+# config file would be "config/raven.yml". If that config doesn't exist,
+# nothing happens.  If it *does*, we register a callback with Canvas::Errors
+# so that every time an exception is reported, we can fire off a sentry
+# call to track it and aggregate it for us.
+settings = ConfigFile.load("raven")
+
+if settings.present?
+  require "raven/base"
+  Raven.configure do |config|
+    config.dsn = settings[:dsn]
+  end
+
+  Canvas::Errors.register!(:sentry_notification) do |exception, data|
+    setting = Setting.get("sentry_error_logging_enabled", 'true')
+    if setting == 'true'
+      if exception.is_a?(String) || exception.is_a?(Symbol)
+        Raven.capture_message(exception, data)
+      else
+        Raven.capture_exception(exception, data)
+      end
+    end
+  end
+end

config/raven.yml.example

@@ -0,0 +1,4 @@
+development:
+  dsn: 'your-sanbox-dsn-here'
+production:
+  dsn: 'your-real-dsn-here'

gems/broadcast_policy/lib/broadcast_policy/instance_methods.rb

@@ -124,8 +124,12 @@ module BroadcastPolicy
         else
           self.workflow_state != self.prior_version.workflow_state
         end
-      rescue Exception => e
+      rescue StandardError => e
-        ErrorReport.log_exception(:broadcast_policy, e, message: "Could not check if a record changed state")
+        Canvas::Errors.capture(
+          e,
+          type: :broadcast_policy,
+          message: "Could not check if a record changed state"
+        )
         logger.warn "Could not check if a record changed state: #{e.inspect}"
         false
       end

gems/twitter/lib/twitter/connection.rb

@@ -77,14 +77,14 @@ module Twitter
             @service.destroy if @service
             @twitter_service.destroy if @twitter_service
           end
-          ErrorReport.log_error(:processing, {
+          Canvas::Errors.capture(:processing, {
-            :backtrace => "Retrieving twitter list for #{@twitter_service.inspect}",
+            backtrace: "Retrieving twitter list for #{@twitter_service.inspect}",
-            :response => response.inspect,
+            response: response.inspect,
-            :body => response.body,
+            body: response.body,
-            :message => response['X-RateLimit-Reset'],
+            message: response['X-RateLimit-Reset'],
-            :url => url
+            url: url
           })
-          retry_after = (response['X-RateLimit-Reset'].to_i - Time.now.utc.to_i) rescue 0 #response['Retry-After'].to_i rescue 0
+          retry_after = (response['X-RateLimit-Reset'].to_i - Time.now.utc.to_i) rescue 0
           raise "Retry After #{retry_after}"
       end
       res

lib/api.rb

@@ -477,9 +477,11 @@ module Api
   end
 
   def self.invalid_time_stamp_error(attribute, message)
-    ErrorReport.log_error('invalid_date_time',
+    Canvas::Errors.capture(
-                          message: "invalid #{attribute}",
+      'invalid_date_time',
-                          exception_message: message)
+      message: "invalid #{attribute}",
+      exception_message: message
+    )
   end
 
   # regex for valid iso8601 dates

lib/canvas.rb

@@ -179,7 +179,7 @@ module Canvas
     raise if options[:raise_on_timeout]
     return nil
   rescue Timeout::Error => e
-    ErrorReport.log_exception(:service_timeout, e)
+    Canvas::Errors.capture_exception(:service_timeout, e)
     raise if options[:raise_on_timeout]
     return nil
   end

lib/canvas/error_stats.rb

@@ -0,0 +1,14 @@
+module Canvas
+  # Simple class for shipping errors to statsd based on the format
+  # propogated from callbacks on Canvas::Errors
+  class ErrorStats
+    def self.capture(exception, _data)
+      category = exception
+      unless exception.is_a?(String) || exception.is_a?(Symbol)
+        category = exception.class.name
+      end
+      CanvasStatsd::Statsd.increment("errors.all")
+      CanvasStatsd::Statsd.increment("errors.#{category}")
+    end
+  end
+end

lib/canvas/errors.rb

@@ -0,0 +1,76 @@
+module Canvas
+  # The central message bus for errors in canvas.
+  #
+  # This class is injected both in ApplicationController for capturing
+  # exceptions that happen in request/response cycles, and in our
+  # Delayed::Job callback for failed jobs.  We also call out to it
+  # from several points throughout the codebase directly to register
+  # an unexpected occurance that doesn't necessarily bubble up to
+  # that point.
+  #
+  # There's a sentry connector built into canvas, but anything one
+  # wants to do with errors can be hooked into this path with the
+  # .register! method.
+  class Errors
+
+    # register something to happen on every exception that occurs.
+    #
+    # The parameter is a unique key for this callback, which is
+    # used when assembling return values from ".capture" and it's friends.
+    #
+    # The block should accept two parameters, one for the exception/message
+    # and one for contextual info in the form of a hash.  The hash *will*
+    # have an ":extra" key, and *may* have a ":tags" key.  tags would
+    # be things it might be useful to aggreate errors around (job queue), extras
+    # are things that would be useful for tracking down why or in what
+    # circumstance an error might occur (request_context_id)
+    #
+    #   Canvas::Errors.register!(:my_key) do |ex, data|
+    #     # do something with the exception
+    #   end
+    def self.register!(key, &block)
+      registry[key] = block
+    end
+
+    # "capture" is the thing to call if you want to tell Canvas::Errors
+    # that something bad happened.  You can pass in an exception, or
+    # just a message.  If you don't build your data hash
+    # with a "tags" key and an "extra" key, it will just group all contextual
+    # information under "extra"
+    def self.capture(exception, data={})
+      run_callbacks(exception, wrap_in_extra(data))
+    end
+
+    def self.capture_exception(type, exception)
+      self.capture(exception, {type: type})
+    end
+
+    # This is really just for clearing out the registry during tests,
+    # if you call it in production it will dump all registered callbacks
+    # that got fired in initializers and such until the process restarts.
+    def self.clear_callback_registry!
+      @registry = {}
+    end
+
+    def self.run_callbacks(exception, extra)
+      registry.each_with_object({}) do |(key, callback), outputs|
+        outputs[key] = callback.call(exception, extra)
+      end
+    end
+    private_class_method :run_callbacks
+
+    def self.registry
+      @registry ||= {}
+    end
+    private_class_method :registry
+
+    def self.wrap_in_extra(data)
+      if data.key?(:tags) || data.key?(:extra)
+        data
+      else
+        {extra: data}
+      end
+    end
+    private_class_method :wrap_in_extra
+  end
+end

lib/canvas/errors/info.rb

@@ -0,0 +1,79 @@
+require_relative '../errors'
+module Canvas
+  class Errors
+
+    # This is a class for taking the common context
+    # found in the request/response cycle for an exception
+    # and turning it into a pleasent hash for Canvas::Errors
+    # to make use of.
+    class Info
+
+      attr_reader :req, :account, :user, :rci, :type
+
+      def initialize(request, root_account, user, opts={})
+        @req = request
+        @account = root_account
+        @user = user
+        @rci = opts.fetch(:request_context_id, RequestContextGenerator.request_id)
+        @type = opts.fetch(:type, nil)
+      end
+
+      # The ideal hash format to pass to Canvas::Errors.capture().
+      #
+      # If you're trying to find a way to transform some other common
+      # context, this is a decent model to follow.
+      def to_h
+        {
+          tags: {
+            account_id: @account.try(:global_id)
+          },
+          extra: {
+            request_context_id: @rci,
+            request_method: @req.request_method_symbol,
+            format: @req.format,
+            user_agent: @req.headers['User-Agent'],
+            user_id: @user.try(:global_id),
+            type: @type
+          }.merge(self.class.useful_http_env_stuff_from_request(@req))
+        }
+      end
+
+      USEFUL_ENV = [
+        "HTTP_ACCEPT",
+        "HTTP_ACCEPT_ENCODING",
+        "HTTP_HOST",
+        "HTTP_REFERER",
+        "HTTP_USER_AGENT",
+        "PATH_INFO",
+        "QUERY_STRING",
+        "REMOTE_HOST",
+        "REQUEST_METHOD",
+        "REQUEST_PATH",
+        "REQUEST_URI",
+        "SERVER_NAME",
+        "SERVER_PORT",
+        "SERVER_PROTOCOL",
+      ].freeze
+
+      def self.useful_http_env_stuff_from_request(req)
+        stuff = req.env.slice(*USEFUL_ENV)
+        req_stuff = stuff.merge(filtered_request_params(req, stuff['QUERY_STRING']))
+        Marshal.load(Marshal.dump(req_stuff))
+      end
+
+      def self.filtered_request_params(req, query_string)
+        f = LoggingFilter
+        {
+          # ActionDispatch::Request#remote_ip has proxy smarts
+          'REMOTE_ADDR' => req.remote_ip,
+          'QUERY_STRING' => (f.filter_query_string("?" + (query_string || ''))),
+          'REQUEST_URI' => f.filter_uri(req.url),
+          'path_parameters' => f.filter_params(req.path_parameters.dup).inspect,
+          'query_parameters' => f.filter_params(req.query_parameters.dup).inspect,
+          'request_parameters' => f.filter_params(req.request_parameters.dup).inspect,
+        }
+      end
+      private_class_method :filtered_request_params
+    end
+  end
+end

lib/canvas/errors/job_info.rb

@@ -0,0 +1,34 @@
+require_relative '../errors'
+module Canvas
+  class Errors
+    class JobInfo
+      def initialize(job, worker)
+        @job = job
+        @worker = worker
+      end
+
+      def to_h
+        {
+          tags: {
+            process_type: "BackgroundJob",
+            job_tag: @job.tag,
+          },
+          extra: extras_hash
+        }
+      end
+
+      private
+      def extras_hash
+        {
+          attempts: @job.attempts,
+          strand: @job.strand,
+          priority: @job.priority,
+          worker_name: @worker.name,
+          handler: @job.handler,
+          run_at: @job.run_at,
+          max_attempts: @job.max_attempts,
+        }
+      end
+    end
+  end
+end

lib/canvas/redis.rb

@@ -63,8 +63,8 @@ module Canvas::Redis
     Rails.logger.error "Failure handling redis command on #{redis_name}: #{e.inspect}"
 
     if self.ignore_redis_failures?
-      ErrorReport.log_exception(:redis, e)
+      Canvas::Errors.capture(e, type: :redis)
-      last_redis_failure[redis_name] = Time.now
+      last_redis_failure[redis_name] = Time.zone.now
       failure_retval
     else
       raise

lib/content_zipper.rb

@@ -48,15 +48,13 @@ class ContentZipper
 
     begin
       case attachment.context
-      when Assignment; zip_assignment(attachment, attachment.context)
+      when Assignment then zip_assignment(attachment, attachment.context)
-      when Eportfolio; zip_eportfolio(attachment, attachment.context)
+      when Eportfolio then zip_eportfolio(attachment, attachment.context)
-      when Folder; zip_base_folder(attachment, attachment.context)
+      when Folder then zip_base_folder(attachment, attachment.context)
-      when Quizzes::Quiz; zip_quiz(attachment, attachment.context)
+      when Quizzes::Quiz then zip_quiz(attachment, attachment.context)
       end
     rescue => e
-      ErrorReport.log_exception(:default, e, {
+      Canvas::Errors.capture(e, message: "Content zipping failed")
-        :message => "Content zipping failed",
-      })
       @logger.debug(e.to_s)
       @logger.debug(e.backtrace.join('\n'))
       attachment.update_attribute(:workflow_state, 'to_be_zipped')

lib/course_link_validator.rb

@@ -25,9 +25,9 @@ class CourseLinkValidator
     validator.check_course(progress)
     progress.set_results({:issues => validator.issues, :completed_at => Time.now.utc})
   rescue
-    report = ErrorReport.log_exception(:course_link_validation, $!)
+    report_id = Canvas::Errors.capture_exception(:course_link_validation, $ERROR_INFO)[:error_report]
     progress.workflow_state = 'failed'
-    progress.set_results({:error_report_id => report.id, :completed_at => Time.now.utc})
+    progress.set_results({error_report_id: report_id, completed_at: Time.now.utc})
   end
 
   attr_accessor :course, :issues, :visited_urls
@@ -206,4 +206,4 @@ class CourseLinkValidator
       false
     end
   end
-end
+end

lib/external_feed_aggregator.rb

@@ -100,11 +100,11 @@ class ExternalFeedAggregator
       feed.increment(:consecutive_failures)
       feed.increment(:failures)
       feed.update_attribute(:refresh_at, Time.now.utc + (FAILURE_WAIT_SECONDS))
-      ErrorReport.log_exception(:default, e, {
+      Canvas::Errors.capture(e, {
-        :message => "External Feed aggregation failed",
+        message: "External Feed aggregation failed",
-        :feed_url => feed.url,
+        feed_url: feed.url,
-        :feed_id => feed.id,
+        feed_id: feed.id,
-        :user_id => feed.user_id,
+        user_id: feed.user_id,
       })
     end
   end

lib/reporting/counts_report.rb

@@ -40,10 +40,11 @@ class CountsReport
   end
 
   def process
-    start_time = Time.now
+    start_time = Time.zone.now
 
     Shackles.activate(:slave) do
-      Shard.with_each_shard(exception: -> { Shard.default.activate { ErrorReport.log_exception(:periodic_job, $!) } }) do
+      callback = -> { Shard.default.activate { Canvas::Errors.capture_exception(:periodic_job, $ERROR_INFO) } }
+      Shard.with_each_shard(exception: callback) do
         Account.root_accounts.active.each do |account|
           next if account.external_status == 'test'
 

lib/send_to_inbox.rb

@@ -59,15 +59,11 @@ module SendToInbox
         end
       end
     rescue => e
-      ErrorReport.log_exception(:default, e, {
+      Canvas::Errors.capture(e, {message: "SendToInbox failure"})
-        :message => "SendToInbox failure",
-      })
       nil
     end
 
-    def inbox_item_recipient_ids
+    attr_reader :inbox_item_recipient_ids
-      @inbox_item_recipient_ids
-    end
 
   end
 

lib/send_to_stream.rb

@@ -57,9 +57,7 @@ module SendToStream
       generate_stream_items(stream_recipients) if stream_recipients
     rescue => e
       if Rails.env.production?
-        ErrorReport.log_exception(:default, e, {
+        Canvas::Errors.capture(e, {message: "SendToStream failure" })
-          :message => "SendToStream failure",
-        })
       else
         raise
       end
@@ -82,19 +80,11 @@ module SendToStream
         true
       end
     rescue => e
-      ErrorReport.log_exception(:default, e, {
+      Canvas::Errors.capture(e, { message: "SendToStream failure" })
-        :message => "SendToStream failure",
-      })
       true
     end
-    
+
-    def generated_stream_items
+    attr_reader :generated_stream_items, :stream_item_recipient_ids
-      @generated_stream_items
-    end
-    
-    def stream_item_recipient_ids
-      @stream_item_recipient_ids
-    end
 
     def stream_item_inactive?
       (self.respond_to?(:workflow_state) && self.workflow_state == 'deleted') || (self.respond_to?(:deleted?) && self.deleted?)

lib/sis/csv/import.rb

@@ -175,11 +175,17 @@ module SIS
         end
       rescue => e
         if @batch
-          error_report = ErrorReport.log_exception(:sis_import, e,
+          message = "Importing CSV for account"\
-            :message => "Importing CSV for account: #{@root_account.id} (#{@root_account.name}) sis_batch_id: #{@batch.id}: #{e.to_s}",
+            ": #{@root_account.id} (#{@root_account.name}) "\
-            :during_tests => false
+            "sis_batch_id: #{@batch.id}: #{e}"
-          )
+          err_id = Canvas::Errors.capture(e,{
-          add_error(nil, I18n.t("Error while importing CSV. Please contact support. (Error report %{number})", number: error_report.id))
+            type: :sis_import,
+            message: message,
+            during_tests: false
+          })[:error_report]
+          error_message = I18n.t("Error while importing CSV. Please contact support."\
+                                 " (Error report %{number})", number: err_id)
+          add_error(nil, error_message)
         else
           add_error(nil, "#{e.message}\n#{e.backtrace.join "\n"}")
           raise e
@@ -259,11 +265,16 @@ module SIS
           importerObject.process(csv)
           run_next_importer(IMPORTERS[IMPORTERS.index(importer) + 1]) if complete_importer(importer)
         rescue => e
-          error_report = ErrorReport.log_exception(:sis_import, e,
+          message = "Importing CSV for account: "\
-            :message => "Importing CSV for account: #{@root_account.id} (#{@root_account.name}) sis_batch_id: #{@batch.id}: #{e.to_s}",
+            "#{@root_account.id} (#{@root_account.name}) sis_batch_id: #{@batch.id}: #{e}"
-            :during_tests => false
+          err_id = Canvas::Errors.capture(e, {
-          )
+            type: :sis_import,
-          add_error(nil, I18n.t("Error while importing CSV. Please contact support. (Error report %{number})", number: error_report.id))
+            message: message,
+            during_tests: false
+          })[:error_report]
+          error_message = I18n.t("Error while importing CSV. Please contact support. "\
+                                 "(Error report %{number})", number: err_id)
+          add_error(nil, error_message)
           @batch.processing_errors ||= []
           @batch.processing_warnings ||= []
           @batch.processing_errors.concat(@errors)
@@ -274,7 +285,7 @@ module SIS
           file.close if file
         end
       end
-    
+
       private
 
       def run_next_importer(importer)

spec/controllers/application_controller_spec.rb

@@ -471,8 +471,10 @@ describe ApplicationController do
       end
 
       it 'should log error reports to the domain_root_accounts shard' do
-        ErrorReport.stubs(:log_exception).returns(ErrorReport.new)
+        report = ErrorReport.new
-        ErrorReport.stubs(:useful_http_env_stuff_from_request).returns({})
+        ErrorReport.stubs(:log_exception).returns(report)
+        ErrorReport.stubs(:find).returns(report)
+        Canvas::Errors::Info.stubs(:useful_http_env_stuff_from_request).returns({})
 
         req = mock()
         req.stubs(:url).returns('url')
@@ -486,7 +488,7 @@ describe ApplicationController do
 
         controller.instance_variable_set(:@domain_root_account, @account)
 
-        @shard2.expects(:activate).twice
+        @shard2.expects(:activate)
 
         controller.send(:rescue_action_in_public, Exception.new)
       end

spec/lib/canvas/error_stats_spec.rb

@@ -0,0 +1,32 @@
+require 'spec_helper'
+
+module Canvas
+  describe ErrorStats do
+    describe ".capture" do
+      before(:each) do
+        CanvasStatsd::Statsd.stubs(:increment)
+      end
+      let(:data){ {} }
+
+      it "increments errors.all always" do
+        CanvasStatsd::Statsd.expects(:increment).with("errors.all")
+        described_class.capture("something", data)
+      end
+
+      it "increments the message name for a string" do
+        CanvasStatsd::Statsd.expects(:increment).with("errors.something")
+        described_class.capture("something", data)
+      end
+
+      it "increments the message name for a symbol" do
+        CanvasStatsd::Statsd.expects(:increment).with("errors.something")
+        described_class.capture(:something, data)
+      end
+
+      it "bumps the exception name for anything else" do
+        CanvasStatsd::Statsd.expects(:increment).with("errors.StandardError")
+        described_class.capture(StandardError.new, data)
+      end
+    end
+  end
+end

spec/lib/canvas/errors/info_spec.rb

@@ -0,0 +1,76 @@
+require 'spec_helper'
+module Canvas
+  class Errors
+    describe Info do
+      let(:request) do
+        stub(env: {}, remote_ip: "", query_parameters: {},
+             request_parameters: {}, path_parameters: {}, url: '',
+             request_method_symbol: '', format: 'HTML', headers: {})
+      end
+
+      let(:request_context_id){ 'abcdefg1234567'}
+      let(:account){ stub(global_id: 1122334455) }
+      let(:user) { stub(global_id: 5544332211)}
+      let(:opts) { { request_context_id: request_context_id }}
+
+      describe 'initialization' do
+        it "grabs the request context id if not provided" do
+          RequestContextGenerator.stubs(:request_id).returns("zzzzzzz")
+          info = described_class.new(request, account, user, {})
+          expect(info.rci).to eq("zzzzzzz")
+        end
+      end
+
+      describe "#to_h" do
+        let(:output) do
+          info = described_class.new(request, account, user, opts)
+          info.to_h
+        end
+
+        it 'digests request information' do
+          request.stubs(:remote_ip).returns("123.456")
+          expect(output[:tags][:account_id]).to eq(1122334455)
+          expect(output[:extra][:request_context_id]).to eq(request_context_id)
+          expect(output[:extra]['REMOTE_ADDR']).to eq("123.456")
+        end
+
+        it "pulls in the request method" do
+          request.stubs(:request_method_symbol).returns("POST")
+          expect(output[:extra][:request_method]).to eq('POST')
+        end
+
+        it 'passes format through' do
+          request.stubs(:format).returns("JSON")
+          expect(output[:extra][:format]).to eq('JSON')
+        end
+
+        it 'includes user information' do
+          expect(output[:extra][:user_id]).to eq(5544332211)
+        end
+
+        it 'passes important headers' do
+          request.stubs(:headers).returns({'User-Agent'=>'the-agent'})
+          expect(output[:extra][:user_agent]).to eq('the-agent')
+        end
+      end
+
+      describe ".useful_http_env_stuff_from_request" do
+        it "duplicates to get away from frozen strings out of the request.env" do
+          dangerous_hash = {
+            "QUERY_STRING".force_encoding(Encoding::ASCII_8BIT).freeze =>
+              "somestuff=blah".force_encoding(Encoding::ASCII_8BIT).freeze,
+            "HTTP_HOST".force_encoding(Encoding::ASCII_8BIT).freeze =>
+              "somehost.com".force_encoding(Encoding::ASCII_8BIT).freeze,
+          }
+          req = stub(env: dangerous_hash, remote_ip: "", url: "",
+                     path_parameters: {}, query_parameters: {}, request_parameters: {})
+          env_stuff = described_class.useful_http_env_stuff_from_request(req)
+          expect do
+            Utf8Cleaner.recursively_strip_invalid_utf8!(env_stuff, true)
+          end.not_to raise_error
+        end
+      end
+
+    end
+  end
+end

spec/lib/canvas/errors/job_info_spec.rb

@@ -0,0 +1,45 @@
+require 'spec_helper'
+
+module Canvas
+  class Errors
+    describe JobInfo do
+      let(:job) do
+        stub(
+          attempts: 1,
+          strand: 'thing',
+          priority: 1,
+          handler: 'Something',
+          run_at: Time.zone.now,
+          max_attempts: 1,
+          tag: "TAG"
+        )
+      end
+
+      let(:worker){ stub(name: 'workername') }
+
+      let(:info){ described_class.new(job, worker) }
+
+      describe "#to_h" do
+        subject(:hash){ info.to_h }
+
+        it "tags all exceptions as 'BackgroundJob'" do
+          expect(hash[:tags][:process_type]).to eq("BackgroundJob")
+        end
+
+        it "includes the tag from the job if there is one" do
+          expect(hash[:tags][:job_tag]).to eq("TAG")
+        end
+
+        it "grabs some common attrs from jobs into extras" do
+          expect(hash[:extra][:attempts]).to eq(1)
+          expect(hash[:extra][:strand]).to eq('thing')
+        end
+
+        it "includes the worker name" do
+          expect(hash[:extra][:worker_name]).to eq('workername')
+        end
+      end
+
+    end
+  end
+end

spec/lib/canvas/errors_spec.rb

@@ -0,0 +1,36 @@
+require 'spec_helper'
+module Canvas
+  describe Errors do
+    before(:each) do
+      described_class.clear_callback_registry!
+    end
+
+    let(:error){ stub("Some Error") }
+
+    it 'fires callbacks when it handles an exception' do
+      called_with = nil
+      Canvas::Errors.register!(:test_thing) do |exception|
+        called_with = exception
+      end
+      Canvas::Errors.capture(error)
+      expect(called_with).to eq(error)
+    end
+
+    it "passes through extra information if available wrapped in extra" do
+      extra_info = nil
+      Canvas::Errors.register!(:test_thing) do |_exception, details|
+        extra_info = details
+      end
+      Canvas::Errors.capture(stub(), {detail1: 'blah'})
+      expect(extra_info).to eq({extra: {detail1: 'blah'}})
+    end
+
+    it 'captures output from each callback according to their registry tag' do
+      Canvas::Errors.register!(:test_thing) do
+        "FOO-BAR"
+      end
+      outputs = Canvas::Errors.capture(stub())
+      expect(outputs[:test_thing]).to eq('FOO-BAR')
+    end
+  end
+end

spec/models/error_report_spec.rb

@@ -35,7 +35,7 @@ describe ErrorReport do
     expect(m).not_to be_nil
     expect(m.to).to eql("nobody@nowhere.com")
   end
-  
+
   it "should not send emails if not configured" do
     account_model
     report = ErrorReport.new
@@ -49,7 +49,8 @@ describe ErrorReport do
   end
 
   it "should not fail with invalid UTF-8" do
-    ErrorReport.log_error('my error', :message => "he\xffllo")
+    data = { extra: { message: "he\xffllo" } }
+    described_class.log_exception_from_canvas_errors('my error', data)
   end
 
   it "should return categories" do
@@ -71,8 +72,9 @@ describe ErrorReport do
   end
 
   it "should use class name for category" do
-    report = ErrorReport.log_exception(nil, e = Exception.new("error"))
+    e = Exception.new("error")
-    expect(report.category).to eq e.class.name
+    report = described_class.log_exception_from_canvas_errors(e, {extra:{}})
+    expect(report.category).to eq(e.class.name)
   end
 
   it "should filter params" do
@@ -88,29 +90,17 @@ describe ErrorReport do
     }
     mock_attrs[:url] = mock_attrs[:env]["REQUEST_URI"]
     req = mock(mock_attrs)
-    report = ErrorReport.new
+    report = described_class.new
-    report.assign_data(ErrorReport.useful_http_env_stuff_from_request(req))
+    report.assign_data(Canvas::Errors::Info.useful_http_env_stuff_from_request(req))
     expect(report.data["QUERY_STRING"]).to eq "?access_token=[FILTERED]&pseudonym[password]=[FILTERED]"
-    expect(report.data["REQUEST_URI"]).to eq "https://www.instructure.example.com?access_token=[FILTERED]&pseudonym[password]=[FILTERED]"
+
+    expected_uri = "https://www.instructure.example.com?"\
+      "access_token=[FILTERED]&pseudonym[password]=[FILTERED]"
+    expect(report.data["REQUEST_URI"]).to eq(expected_uri)
     expect(report.data["path_parameters"]).to eq({ :api_key => "[FILTERED]" }.inspect)
-    expect(report.data["query_parameters"]).to eq({ "access_token" => "[FILTERED]", "pseudonym[password]" => "[FILTERED]" }.inspect)
+    q_params = { "access_token" => "[FILTERED]", "pseudonym[password]" => "[FILTERED]" }
+    expect(report.data["query_parameters"]).to eq(q_params.inspect)
     expect(report.data["request_parameters"]).to eq({ "client_secret" => "[FILTERED]" }.inspect)
   end
 
-  describe ".useful_http_env_stuff_from_request" do
-    it "duplicates to get away from frozen strings out of the request.env" do
-      dangerous_hash = {
-        "QUERY_STRING".force_encoding(Encoding::ASCII_8BIT).freeze =>
-          "somestuff=blah".force_encoding(Encoding::ASCII_8BIT).freeze,
-        "HTTP_HOST".force_encoding(Encoding::ASCII_8BIT).freeze =>
-          "somehost.com".force_encoding(Encoding::ASCII_8BIT).freeze,
-      }
-      req = stub(env: dangerous_hash, remote_ip: "", url: "",
-                 path_parameters: {}, query_parameters: {}, request_parameters: {})
-      env_stuff = ErrorReport.useful_http_env_stuff_from_request(req)
-      expect{
-        Utf8Cleaner.recursively_strip_invalid_utf8!(env_stuff, true)
-      }.not_to raise_error
-    end
-  end
 end