Add per-exception handling configuration

[andrewclink]

This PR solves two problems.

Rather than get thousands of 404 emails every day aimed at wp-admin.php and such, I decided to add some options to direct handling of these errors.

In the case that the canned exception layout isn't desired, a layout must be supplied for each status code. Hardcoding the same thing for each status code didn't seem quite right, nor was I interested in getting bogged down with which HTTP codes are kosher to ignore. I initially handled this by configuring with Hash[*(500..599).map{|x| [x, nil] }.flatten] but that's not the right way to handle this either.

My solution is to move away from the layouts config option and rename it to exceptions (I'm happy to call this anything else too) in order to provide a place that each status code can be configured to be handled differently. For example, I want 500 errors delivered, with my 'something_broke' layout, but I don't want notification of 404 errors unless they are an image:

config.exception_handler = {
    email: 'email@example.com',
    exceptions: {
        :all => { layout: 'something_broke' },
         404  => { 
            deliver: proc {|e| e.request.original_fullpath =~ /jpg|png/ },
         },
    }
}

Tests were performed, but I didn't see any test suite in the repo so I wasn't able to write any. I also gave a quick glance at the 0.8 branch and saw that the configuration options look unchanged, so hopefully this is simple enough to apply there as well.

[richpeck]

Thanks for the update, let me have a look into this

[richpeck]

In the case that the canned exception layout isn't desired, a layout must be supplied for each status code. Hardcoding the same thing for each status code didn't seem quite right.

Agreed (originally considered this).

The only issue I had is that the entire system is based around HTTP_STATUS_CODES.

This meant that I had to ensure that all codes could be represented. Although I personally wanted a simple way to switch 40x and 50x errors, I felt it appropriate to make the system as granular as possible.

--

My solution is to move away from the layouts config option and rename it to exceptions

Good idea, especially considering the different functionality you're proposing.

In terms of the use value of this, I am seeing that you're looking at including several options within each HTTP error (layout + deliver). The deliver thing maybe. You're basically doing a per-error bool on whether reports should be sent to the listed email. You'd be better calling it email because I originally thought deliver would be shorthand for error-specific responses (which would be interesting).

In terms of the layout, the functionality remains the same, except you've included an all option. This is okay, but you've got the same issue that I had - trying to come up with a succinct way to define the various layouts (for all, 40x & 50x). I was going to just call them 4xx or 5xx but I wanted it to be as simple as possible...

config.exception_handler = {
  layouts: { 4xx: 'test', 5xx: 'test'}
}        

--

In regard to your overall proposal, I think the BIG thing we could work with would be the renaming of layout to exceptions and then including multiple options for each error (layout, email/notify, deliver):

config.exception_handler = {
    email: 'email@example.com',
    exceptions: {
        all: { layout: 'something_broke' },
        5xx: { layout: 'test' },
        404: {
            layout: 'test', 
            notify:  proc {|e| e.request.original_fullpath =~ /jpg|png/ },
            deliver: #something here to control the type of response
        }
    }
}

This would give even greater configuration management to the errors, whilst retaining the overall structure & veracity of the solution.

[richpeck]

Closing as I've incorporated into the next version (0.8.0.0) - I've credited you etc.

[andrewclink]

Cool, thanks!

On 25 Jun, 2018, at 11:36 PM, Richard Peck notifications@github.com wrote:

Closing as I've incorporated into the next version (0.8.0.0) - I've credited you etc.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/richpeck/exception_handler/pull/63#issuecomment-400195635, or mute the thread https://github.com/notifications/unsubscribe-auth/AAqDFOcKVlwQglsPsD183nD2_jGHZPpUks5uAdZxgaJpZM4T9AU8.