lessonly / scim_rails

SCIM Adapter for Rails.
MIT License
68 stars 76 forks source link

Add logging for unhandled exceptions #27

Closed rreinhardt9 closed 4 years ago

rreinhardt9 commented 4 years ago

Why?

resolves lessonly/scim_rails#23 fixes lessonly/scim_rails#25 fixes lessonly/scim_rails#26

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.

This also fixes some pains encountered during development with running tasks and commands

Testing Notes

This one is a bit complicated to test because the exception catching behavior is only configured to happen in production environments.

You can make sure that the existing spec pass, and to see the exception handling in action you can pull down the code and make the following modifications:

Run the test suite now... you should see failures related to the response not matching and you should see the output from the puts.

Merge Instructions

Please rebase these commits when merging

rreinhardt9 commented 4 years ago

@wernull I think you've had experience with working on this gem in the past so I tagged you as reviewer; would you mind looking at this with me?

rreinhardt9 commented 4 years ago

Thanks for the review and merge @wernull !!!! 🙏 🎉 :shipit: