Implement client error logging

.travis.yml

@@ -5,6 +5,9 @@ rvm:
   - 2.2
   - 2.3.0
 
+env:
+  - RUBYOPT="-W0"
+
 gemfile:
   - gemfiles/Gemfile.rails32
   - gemfiles/Gemfile.rails41

app/assets/javascripts/ndr_error/client_errors.js

@@ -0,0 +1,36 @@
+var NdrError = {
+  url: function() {
+    var origin = window.location.origin;
+
+    // IE doesn't support `location.origin`:
+    origin = origin || (
+      window.location.protocol + '//' +
+      window.location.hostname +
+      (window.location.port ? ':' + window.location.port: '')
+    );
+
+    // TODO: '/fingerprinting' is configureable, use
+    // client_errors_url helper?
+    return origin + '/fingerprinting/client_errors';
+  },
+
+  notify: function(message, source, lineno, colno, error) {
+    jQuery.post(NdrError.url(), {
+      'client_error': {
+        'message': message,
+        'source':  source,
+        'lineno':  lineno,
+        'colno':   colno,
+        'stack':   error && error.stack,
+        'window.width':  window.innerWidth,
+        'window.height': window.innerHeight,
+        'screen.width':  window.screen.width,
+        'screen.height': window.screen.height
+      }
+    })
+  }
+};
+
+window.onerror = function(message, source, lineno, colno, error) {
+  NdrError.notify(message, source, lineno, colno, error);
+}

app/assets/javascripts/ndr_error/ndr_error.js

@@ -13,6 +13,7 @@
 //= require jquery
 //= require jquery_ujs
 //= require ndr_error/bootstrap/bootstrap
+//= require ndr_error/client_errors
 
 jQuery(function() {
   // Backtrace toggling:
@@ -46,6 +47,7 @@ jQuery(function() {
     jQuery('.badge').tooltip();
 
     $searchField.keydown(function(event) {
+      wenewnew
       // <ENTER> will submit the search form.
       if (event.keyCode == 13) {
         this.form.submit();
@@ -58,3 +60,15 @@ jQuery(function() {
     });
   })();
 });
+
+window.onerror = function(message, source, lineno, colno, error) {
+  jQuery.post('client_errors/', {
+    'client_error': {
+      'message': message,
+      'source':  source,
+      'lineno':  lineno,
+      'colno':   colno,
+      'stack':   error && error.stack
+    }
+  })
+}

app/controllers/ndr_error/client_errors_controller.rb

@@ -0,0 +1,15 @@
+module NdrError
+  # Controller for receiving client errors
+  class ClientErrorsController < ApplicationController
+    def create
+      exception        = JavascriptError.new(params[:client_error])
+      ancillary        = {} # TODO: populate this as the middleware does
+      fingerprint, log = NdrError.log(exception, ancillary, request)
+
+      render json: {
+        fingerprint: fingerprint.id,
+        uuid: log.try(:id)
+      }
+    end
+  end
+end

app/models/ndr_error/log.rb

@@ -116,13 +116,15 @@ def register_exception(exception)
       self.error_class = exception.class.to_s
       self.backtrace   = exception.backtrace
       self.description = description_from(exception.message)
+
+      self.parameters_yml = exception.metadata if client_error?
     end
 
     # Stores parameters from the given _request_ object
     # as YAML. Will attempt to store as many as possible
     # of the parameters in the available 4000 chars.
     def register_request(request)
-      extract_request_params(request)
+      extract_request_params(request) unless client_error?
       extract_request_attributes(request)
     end
 
@@ -151,7 +153,7 @@ def parameters_yml=(params)
     # Returns the params hash associated
     # with the request.
     def parameters
-      YAML.load(parameters_yml)
+      YAML.safe_load(parameters_yml, [Symbol])
     end
 
     # Formats error to be like the ruby error.
@@ -176,6 +178,10 @@ def md5_digest=(digest)
       @_digest = digest
     end
 
+    def client_error?
+      error_class == 'NdrError::JavascriptError'
+    end
+
     private
 
     # For the given `request' object, return the
@@ -199,9 +205,11 @@ def extract_request_params(request)
     def extract_request_attributes(request)
       return unless request
 
+      uri = request.env[client_error? ? 'HTTP_REFERER' : 'REQUEST_URI']
+
       self.port       = request.env['SERVER_PORT']
       self.ip         = "#{request.env['REMOTE_ADDR']}/#{request.remote_ip}"
-      self.url        = "#{request.env['REQUEST_URI']} (on #{request.host})"
+      self.url        = "#{uri} (on #{request.host})"
       self.user_agent = request.env['HTTP_USER_AGENT']
     end
 

config/routes.rb

@@ -3,4 +3,8 @@
             only: [:index, :show, :edit, :update, :destroy],
             controller: 'errors',
             as: 'error_fingerprints'
+
+  resources :client_errors,
+            only: [:create],
+            controller: 'client_errors'
 end

lib/ndr_error.rb

@@ -3,6 +3,7 @@
 require 'ndr_error/backtrace_compression'
 require 'ndr_error/finder'
 require 'ndr_error/fuzzing'
+require 'ndr_error/javascript_error'
 require 'ndr_error/logging'
 require 'ndr_error/uuid_builder'
 

lib/ndr_error/fuzzing.rb

@@ -17,6 +17,7 @@ def fuzz_description(description)
     #   * independent of deployment paths
     #   * independent of line numbers
     def fuzz_backtrace(backtrace)
+      return '' if client_error?
       backtrace.map { |line| fuzz_line(line) }.join("\n")
     end
 

lib/ndr_error/javascript_error.rb

@@ -0,0 +1,25 @@
+module NdrError
+  # Class to wrap / normalise Javascript exception
+  # data, and allow it to be logged by NdrError.
+  class JavascriptError < Exception
+    attr_reader :source
+
+    def initialize(parameters)
+      @source = parameters.with_indifferent_access
+
+      super(@source['message'])
+
+      set_backtrace_from_stack
+    end
+
+    def metadata
+      source.except('message', 'stack')
+    end
+
+    private
+
+    def set_backtrace_from_stack
+      set_backtrace @source.fetch('stack', '').split("\n")
+    end
+  end
+end

test/dummy/app/assets/javascripts/application.js

@@ -12,4 +12,5 @@
 //
 //= require jquery
 //= require jquery_ujs
+//= require ndr_error/client_errors
 //= require_tree .

test/dummy/app/controllers/disaster_controller.rb

@@ -1,9 +1,15 @@
 # Some application logic that we should be able to log failures from:
 class DisasterController < ApplicationController
   def no_panic
+    # Triggers no exceptions
   end
 
   def cause
+    # Triggers a server-side exception
     fail params[:message]
   end
+
+  def cause_client
+    # Triggers a client-side exception
+  end
 end

test/dummy/app/views/disaster/cause_client.html.erb

@@ -0,0 +1,4 @@
+<p>This page should raise a Javascript error</p>
+<script type="text/javascript" charset="utf-8">
+  1 + fooBarBaz;
+</script>

test/dummy/config/environments/development.rb

@@ -10,7 +10,7 @@
   config.whiny_nils = true
 
   # Show full error reports and disable caching
-  config.consider_all_requests_local       = true
+  config.consider_all_requests_local       = false
   config.action_controller.perform_caching = false
 
   # Don't care if the mailer can't send

test/integration/client_error_logging_test.rb

@@ -0,0 +1,22 @@
+require 'test_helper'
+
+# Ensure host app is able to log errors sent to the API endpoint
+class ClientErrorLoggingTest < ActionDispatch::IntegrationTest
+  def setup
+    NdrError::Fingerprint.delete_all
+    NdrError::Log.delete_all
+  end
+
+  test 'should log client exceptions' do
+    assert_difference(-> { NdrError::Log.count }, 2) do
+      2.times { visit '/disaster/cause_client' }
+    end
+
+    error_logs = NdrError::Log.all
+    assert error_logs.map(&:error_fingerprint).uniq.one?
+    error_log = error_logs.first
+
+    assert_equal 'NdrError::JavascriptError', error_log.error_class
+    assert_equal "ReferenceError: Can't find variable: fooBarBaz", error_log.description
+  end
+end

test/test_helper.rb

@@ -33,8 +33,9 @@
 Capybara.register_driver :poltergeist do |app|
   options = {
     # debug: true, # Uncomment for more verbose
-    # inspector: true, # DEBUGGING suppport.
+    inspector: true, # DEBUGGING suppport.
     phantomjs_options: ['--proxy-type=none'],
+    js_errors: false,
     timeout: 60
   }
 

test/unit/ndr_error/fuzzing_test.rb

@@ -3,58 +3,100 @@
 module NdrError
   # Test our fuzzing / digest creation:
   class FuzzingTest < ActiveSupport::TestCase
-    include NdrError::Fuzzing
+    # Dummy class for testing Fuzzing mixin
+    class Fuzzable
+      include NdrError::Fuzzing
+
+      def client_error?
+        false
+      end
+    end
+
+    def setup
+      @fuzzable = Fuzzable.new
+    end
 
     test 'should fuzz descriptions correctly' do
       description = "TemplateError: undefined method `clean' for [\"XYZ\", \"ABC\"]:Array"
-      refute send(:fuzz_description, description).include?('XYZ')
+      refute fuzz_description(description).include?('XYZ')
 
       # Should obfuscate objectids:
       assert_equal fuzz('#<EUserBatch:0x123e12345>', []), fuzz('#<EUserBatch:0x134a45639>', [])
     end
 
     test 'should fuzz Rails root directory from backtraces' do
-      trace = send(:fuzz_backtrace, [Rails.root.join('app').to_s])
+      trace = fuzz_backtrace([Rails.root.join('app').to_s])
       assert_equal 'Rails.root/app', trace
     end
 
     test 'should fuzz gem differences from backtraces' do
       # Should fuzz gem paths:
-      trace = send(:fuzz_backtrace, [Gem.path.first + '/app'])
+      trace = fuzz_backtrace([Gem.path.first + '/app'])
       assert_equal 'Gem.path/app', trace
 
       # Should remove gem version number when fuzzing gem paths
       line = Gem.path.first + "/gems/evil-1.4.3/lib/evil/file.rb:623:in `method'"
-      assert_equal "Gem.path/gems/evil-/lib/evil/file.rbin `method'", send(:fuzz_backtrace, [line])
+      assert_equal "Gem.path/gems/evil-/lib/evil/file.rbin `method'", fuzz_backtrace([line])
     end
 
     test 'should fuzz LOAD_PATH from backtraces' do
       # Should fuzz load path entries:
-      trace = send(:fuzz_backtrace, [$LOAD_PATH.first + '/app'])
+      trace = fuzz_backtrace([$LOAD_PATH.first + '/app'])
       refute trace.include?($LOAD_PATH.first)
     end
 
     test 'should fuzz line numbers from backtraces' do
       # Should fuzz line numbers:
-      trace = send(:fuzz_backtrace, ['app/myfile.rb:12: in function'])
+      trace = fuzz_backtrace(['app/myfile.rb:12: in function'])
       assert_equal 'app/myfile.rb in function', trace
     end
 
     test 'should fuzz compiled template / partial IDs from backtraces' do
       template = ActionView::Template.new('test template', 'template.html.erb', nil, {})
       compiled = template.send(:method_name) # The method name of the template once compiled
 
-      assert_equal '_template_html_erb__COMPILED_ID', send(:fuzz_backtrace, [compiled])
+      assert_equal '_template_html_erb__COMPILED_ID', fuzz_backtrace([compiled])
 
       partial  = ActionView::Template.new('test partial', '_partial.html.erb', nil, {})
       compiled = partial.send(:method_name) # The method name of the partial once compiled
 
-      assert_equal '__partial_html_erb__COMPILED_ID', send(:fuzz_backtrace, [compiled])
+      assert_equal '__partial_html_erb__COMPILED_ID', fuzz_backtrace([compiled])
     end
 
     test 'should fuzz compiled callbacks from backtraces' do
-      trace = send(:fuzz_backtrace, ['_run__2058915813__process_action__1931044129__callbacks'])
+      trace = fuzz_backtrace(['_run__2058915813__process_action__1931044129__callbacks'])
       assert_equal '_run__COMPILED_ID__process_action__COMPILED_ID__callbacks', trace
     end
+
+    test '#fuzz_backtrace should be consistent with client errors' do
+      @fuzzable.stubs(client_error?: true)
+      assert_equal fuzz_backtrace(['abc']), @fuzzable.send(:fuzz_backtrace, ['bcd'])
+    end
+
+    test 'fuzzing should be sensitive to client error descriptions' do
+      @fuzzable.stubs(client_error?: true)
+      assert_equal fuzz('test', []), fuzz('test', [])
+      refute_equal fuzz('test', []), fuzz('zest', [])
+    end
+
+    test 'fuzzing should be not sensitive to client error backtraces' do
+      @fuzzable.stubs(client_error?: true)
+      assert_equal fuzz('test', %w(t e s t)), fuzz('test', %w(t e s t))
+      assert_equal fuzz('test', %w(t e s t)), fuzz('test', %w(e s t e))
+    end
+
+    private
+
+    def fuzz_description(string)
+      @fuzzable.send(:fuzz_description, string)
+    end
+
+    def fuzz_backtrace(trace)
+      @fuzzable.send(:fuzz_backtrace, trace)
+    end
+
+    def fuzz(description, backtrace)
+      @fuzzable.fuzz(description, backtrace)
+    end
   end
 end

test/unit/ndr_error/javascript_error_test.rb

@@ -0,0 +1,20 @@
+require 'test_helper'
+
+module NdrError
+  # Unit Test Log.
+  class JavaScriptErrorTest < ActiveSupport::TestCase
+    test 'should set backtrace' do
+      params = {
+        'message' => 'ReferenceError: adfgrdfgkljh is not defined',
+        'source'  => 'http://localhost:3000',
+        'lineno'  => '49',
+        'colno'   => '1',
+        'stack'   => "stack_values\nline 1\nline 2\nline 3"
+      }
+      error = JavascriptError.new(params)
+
+      assert_equal 'ReferenceError: adfgrdfgkljh is not defined', error.message
+      assert_equal 4, error.backtrace.length
+    end
+  end
+end