From b6f156e01e3beadd005aaf1d62873c64783e8a64 Mon Sep 17 00:00:00 2001 From: Ross Reinhardt Date: Fri, 29 May 2020 16:32:10 -0400 Subject: [PATCH 1/2] Fix tasks and commands so they can be run from the root directory fixes lessonly/scim_rails#25 fixes lessonly/scim_rails#26 Why Since we are using the spec/ director instead of the default test/ we need to specify the locations of the dummy application and it's rake file. With out this, you cannot run things like db:migrate from the root of the engine as you should. This goes for any rake task you would usually call from the root. Additionally, commands like rails console would not work because it would not be able to find the dummy application to load it. --- Rakefile | 2 +- bin/rails | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index d390a46a..435cbb33 100644 --- a/Rakefile +++ b/Rakefile @@ -14,7 +14,7 @@ RDoc::Task.new(:rdoc) do |rdoc| rdoc.rdoc_files.include('lib/**/*.rb') end -APP_RAKEFILE = File.expand_path("../test/dummy/Rakefile", __FILE__) +APP_RAKEFILE = File.expand_path("../spec/dummy/Rakefile", __FILE__) load 'rails/tasks/engine.rake' diff --git a/bin/rails b/bin/rails index 8c9e7642..a1c2ca26 100755 --- a/bin/rails +++ b/bin/rails @@ -4,6 +4,7 @@ ENGINE_ROOT = File.expand_path('../..', __FILE__) ENGINE_PATH = File.expand_path('../../lib/scim_rails/engine', __FILE__) +APP_PATH = File.expand_path('../../spec/dummy/config/application', __FILE__) # Set up gems listed in the Gemfile. ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../../Gemfile', __FILE__) From 7ba7da5c14123aab93ea2f5d2fd365e5c0150c06 Mon Sep 17 00:00:00 2001 From: Ross Reinhardt Date: Fri, 29 May 2020 16:47:57 -0400 Subject: [PATCH 2/2] Add ability to supply a callable error handler resolves lessonly/scim_rails#23 Why? The scim engine returns a custom response to the caller in the event that there is a 500 error in the engine. It rescues any standard error in order to do this. Unfortunately this means that when something is broken it does not bubble up to the parent app's error handling system or even be printed in the logs. We cannot just re-raise the exception because you cannot return a response AND raise an exception as a part of the same request. To help, this PR will make is possible for you to supply a callable object that take the exception as it's argument. If no callable object is provided, we will output the exception to the logs so that silence is not the default. To get the old behavior of completely ignoring an exception, you could supply an empty proc. --- .gitignore | 1 + .rubocop.yml | 2 ++ CHANGELOG.md | 3 +++ README.md | 14 ++++++++++++++ .../concerns/scim_rails/exception_handler.rb | 13 ++++++++----- lib/scim_rails/config.rb | 16 +++++++++++----- 6 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 .rubocop.yml create mode 100644 CHANGELOG.md diff --git a/.gitignore b/.gitignore index aae7f11f..6b7fe9d1 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ spec/dummy/db/*.sqlite3 spec/dummy/db/*.sqlite3-journal spec/dummy/log/*.log spec/dummy/tmp/ +.rubocop-https* diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 00000000..6651a65c --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,2 @@ +inherit_from: + - https://raw.githubusercontent.com/lessonly/rubocop-default-configuration/master/.rubocop.yml diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 00000000..84f8feeb --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,3 @@ +# Upcoming Release + +- Any unhandled error is now logged to the configured rails logger by default. You can also now supply a custom callable that will be used to handle those exceptions instead. diff --git a/README.md b/README.md index 10b1863c..9a59afa7 100644 --- a/README.md +++ b/README.md @@ -246,6 +246,20 @@ Sample request: $ curl -X PATCH 'http://username:password@localhost:3000/scim/v2/Users/1' -d '{"schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], "Operations": [{"op": "replace", "value": { "active": false }}]}' -H 'Content-Type: application/scim+json' ``` +### Error Handling + +By default, scim_rails will output any unhandled exceptions to your configured rails logs. + +If you would like, you can supply a custom handler for exceptions in the initializer. The only requirement is that the value you supply responds to `#call`. + +For example, you might want to notify Honeybadger: + +```ruby +ScimRails.configure do |config| + config.on_error = ->(e) { Honeybadger.notify(e) } +end +``` + ## Contributing ### [Code of Conduct](https://github.com/lessonly/scim_rails/blob/master/CODE_OF_CONDUCT.md) diff --git a/app/controllers/concerns/scim_rails/exception_handler.rb b/app/controllers/concerns/scim_rails/exception_handler.rb index 2adf1909..f87cbc0a 100644 --- a/app/controllers/concerns/scim_rails/exception_handler.rb +++ b/app/controllers/concerns/scim_rails/exception_handler.rb @@ -12,12 +12,15 @@ class UnsupportedPatchRequest < StandardError end included do - # StandardError must be ordered _first_ or it will catch all exceptions - # - # TODO: Build a plugin/configuration for error handling so that the - # detailed production errors are logged somewhere if desired. if Rails.env.production? - rescue_from StandardError do + rescue_from StandardError do |exception| + on_error = ScimRails.config.on_error + if on_error.respond_to?(:call) + on_error.call(exception) + else + Rails.logger.error(exception.inspect) + end + json_response( { schemas: ["urn:ietf:params:scim:api:messages:2.0:Error"], diff --git a/lib/scim_rails/config.rb b/lib/scim_rails/config.rb index 5d022b3e..aa4f108b 100644 --- a/lib/scim_rails/config.rb +++ b/lib/scim_rails/config.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ScimRails class << self def configure @@ -5,22 +7,26 @@ def configure end def config - @_config ||= Config.new + @config ||= Config.new end end + # Class containing configuration of ScimRails class Config - ALGO_NONE = "none".freeze + ALGO_NONE = "none" - attr_accessor \ + attr_writer \ :basic_auth_model, + :mutable_user_attributes_schema, + :scim_users_model + + attr_accessor \ :basic_auth_model_authenticatable_attribute, :basic_auth_model_searchable_attribute, :mutable_user_attributes, - :mutable_user_attributes_schema, + :on_error, :queryable_user_attributes, :scim_users_list_order, - :scim_users_model, :scim_users_scope, :scim_user_prevent_update_on_create, :signing_secret,