Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Capture more information on exception #3

Closed
nlsrchtr opened this issue Mar 15, 2012 · 15 comments
Closed

Capture more information on exception #3

nlsrchtr opened this issue Mar 15, 2012 · 15 comments

Comments

@nlsrchtr
Copy link

Hi @Sharagoz,

I really like your exception notification solution, because it's offering the possibility to send the exception to a central URI. This makes handling the exceptions a lot easier than getting spamed via email.

But compared with other exception notifier, I'm only getting limited data to track down a bug. I get a whole backtrace and some information on the request params. But for example to track a bug in a load-balanced scenario, I can't figure out on which server the exception occurs.

Are there any plans to add some more detailed information, like f.e. exception notification gem (https://github.com/smartinez87/exception_notification) it is offering: request, session, environment and backtrace.


-------------------------------
Request:
-------------------------------

 * URL       : 
 * IP address: 
 * Parameters: 
 * Rails root: 

-------------------------------
Session:
-------------------------------

 * session id: 
 * data: 

-------------------------------
Environment:
-------------------------------

 * DOCUMENT_ROOT                               : 
 * HTTPS                                       : 
 * HTTP_ACCEPT                                 : 
 * HTTP_ACCEPT_CHARSET                         : 
 * HTTP_ACCEPT_ENCODING                        : 
 * HTTP_ACCEPT_LANGUAGE                        : 
 * HTTP_CONNECTION                             : 
 * HTTP_COOKIE                                 : 
 * HTTP_HOST                                   : 
 * HTTP_REFERER                                : 
 * HTTP_USER_AGENT                             : 
 * PASSENGER_CONNECT_PASSWORD                  : 
 * PATH_INFO                                   : 
 * QUERY_STRING                                : 
 * REMOTE_ADDR                                 : 
 * REMOTE_PORT                                 : 
 * REQUEST_METHOD                              : 
 * REQUEST_URI                                 : 
 * SCRIPT_NAME                                 : 
 * SERVER_ADDR                                 : 
 * SERVER_ADMIN                                : 
 * SERVER_NAME                                 : 
 * SERVER_PORT                                 : 
 * SERVER_PROTOCOL                             : 
 * SERVER_SOFTWARE                             : 
 * UNIQUE_ID                                   : 
 * _                                           : 
 * action_controller.request.path_parameters   : 
 * action_controller.request.query_parameters  : 
 * action_controller.request.request_parameters: 
 * action_controller.rescue.request            : 
 * action_controller.rescue.response           : 
 * rack.errors                                 : 
 * rack.input                                  : 
 * rack.multiprocess                           : 
 * rack.multithread                            : 
 * rack.request.cookie_hash                    : 
 * rack.request.cookie_string                  : 
 * rack.request.query_hash                     : 
 * rack.request.query_string                   : 
 * rack.run_once                               : 
 * rack.session                                : 
 * rack.session.options                        : 
 * rack.session.record                         : 
 * rack.url_scheme                             : 
 * rack.version                                : 

 * Process:
 * Server :

-------------------------------
Backtrace:
-------------------------------

  [...]
@bjorntrondsen
Copy link
Owner

I dont have any concrete plans to extend what is logged because nobody has requested it yet. I'm open for doing it though.

If I am going to mirror the information extracted by the exception notification gem, the two projects might as well be merged. Do you need all those paramterers for my gem to solve your needs, or are there only a few things you're missing?

@nlsrchtr
Copy link
Author

Sounds great! It would be good to have as many information as possible, because depending on the issue some information are worth more than others. The mix from the exception notification gem was always quite good working for me.

Maybe you could offer a config option, like:
config.exception_details = [:request, :session, :environment, :backtrace]
which defaults to [:backtrace] so currently working installations aren't bothered with this option, otherwise they also would need to migrate the database.

@bjorntrondsen
Copy link
Owner

Yeah, if I'm gonna add that many new params I definitely need to enable configuration of which params to store.
I'll look closer into what's involved this weekend and get back to you.

@nlsrchtr
Copy link
Author

Just a quick thought on the implementation: maybe it would make sense to store the values in the new key-value storage store. So it would be more flexible and extendable.

But on the downside it would limit the access to this feature to Rails 3.2 applications only.

@bjorntrondsen
Copy link
Owner

Hello again Niels

I've come up with a new API which uses blocks that should enable everybody to store the information they need. It will look something like this:

config.store_request_info do |storage,request|
  storage[:target_url] =    request.url
  storage[:referer_url] =   request.referer
  storage[:params] =        request.params.inspect
  storage[:user_agent] =    request.user_agent
end

config.store_exception_info do |storage,exeception|
  storage[:class_name] =   exception.class.to_s
  storage[:message] =      exception.to_s
  storage[:trace] =        exception.backtrace.join("\n")
end

config.store_environment_info do |storage,env|
  storage[:doc_root] =      env['DOCUMENT_ROOT']
  storage[:rack_errors] =   env['rack.errors']
  storage[:ac_path_params]  env['action_controller.request.path_parameters']
end

config.store_global_info do |storage|
  storage[:app_name] =     Rails.application.class.parent_name
  storage[:created_at] =   Time.now
end

It gives you direct access to the env, request and exception objects, so you can extract whatever you need. There will be more configuration to do when setting up the exception handler the first time, but with good documentation I dont think that should be much of a problem for anybody. I think this will be very simple to implement. I will probably spend a lot more time writing the documentation than writing the implementation.

Regarding key-val storage: I wont hard code the usage of key-value storage when the exception handler is set up to store through ActiveRecord, but it's actually very simple to configure (monkey patch) the AR model to do this:

With the new API you can store a group of attributes in the same key, and then tell the AR model to use the new rails 3.2 storage method for this key like so:

config.store_environment_info do |storage,env|
  storage[:env_info][:rack_errors] =    env['rack.errors']
  storage[:env_info][:ac_path_params]   env['action_controller.request.path_parameters']
 # etc..
end
config.after_initialize do
  ErrorMessage.send(:store, :env_info)
end

I'm excited about this new approach. What do you think?

@nlsrchtr
Copy link
Author

Looks great. Very good solution!

If this would be the default behavior, it could me sense to offer some presets of parameters (:small, :normal, :extended) and the option to overwrite completely. So not everybody would have to hassle to get the set of parameters they need, but have the possibility.

Looking forward to your solution!

@bjorntrondsen
Copy link
Owner

The new API is implemented. You can take it for a spin if you want. I havent released a gem yet as I need to add more specs and documentation first, but you can pull it from master for now:

gem 'rails_exception_handler', :git => 'git://github.com/Sharagoz/rails_exception_handler.git', :ref => '523adccf3c1c13a167fa24ecab374b7d1bc01e27'

The new API will break backwards compatibitlity, so I will be bumping the version to 2.0. Here's the configuration lines needed to make it backwards compatible:

config.store_request_info do |storage,request|
  storage[:target_url] =  request.url
  storage[:referer_url] = request.referer
  storage[:params] =      request.params.inspect
  storage[:user_agent] =  request.user_agent
end
config.store_exception_info do |storage,exception|
  storage[:class_name] =   exception.class.to_s
  storage[:message] =      exception.to_s
  storage[:trace] =        exception.backtrace.join("\n")
end
config.store_environment_info do |storage,env|
  # Not in use in v1.0, leave it out if you want
end
config.store_global_info do |storage|
  storage[:app_name] =     Rails.application.class.parent_name
  storage[:created_at] =   Time.now
end

@nlsrchtr
Copy link
Author

nlsrchtr commented Apr 1, 2012

Hi Sharagoz,

sorry for the delay. Two busy weeks lying behind me now. But today, I added the new release to one of my projects and had the following problem.

With:

  config.store_request_info do |storage,request|
    storage[:target_url] = request.url
    storage[:referer_url] = request.referer
    storage[:params] = request.params.inspect
    storage[:user_agent] = request.user_agent
  end

  config.store_exception_info do |storage,exception|
    storage[:class_name] = exception.class.to_s
    storage[:message] = exception.to_s
    storage[:trace] = exception.backtrace.join("\n")
  end

  config.store_environment_info do |storage,env|
    storage[:doc_root] = env['DOCUMENT_ROOT']
    storage[:rack_errors] = env['rack.errors']
    storage[:ac_path_params] = env['action_controller.request.path_parameters']
  end

  config.store_global_info do |storage|
    storage[:app_name] = Rails.application.class.parent_name
    storage[:created_at] = Time.now
  end

in the initializer, I only got on the server side

Parameters: {"error_message"=>"[:user_info, \"Anonymous\"]"}

and no more information. Than I tried to scope the attributes, like:

  config.store_request_info do |storage,request|
    storage[:request_info][:target_url] = request.url
    storage[:request_info][:referer_url] = request.referer
    storage[:request_info][:params] = request.params.inspect
    storage[:request_info][:user_agent] = request.user_agent
  end

  config.store_exception_info do |storage,exception|
    storage[:exception_info][:class_name] = exception.class.to_s
    storage[:exception_info][:message] = exception.to_s
    storage[:exception_info][:trace] = exception.backtrace.join("\n")
  end

  config.store_environment_info do |storage,env|
    storage[:environment_info][:doc_root] = env['DOCUMENT_ROOT']
    storage[:environment_info][:rack_errors] = env['rack.errors']
    storage[:environment_info][:ac_path_params] = env['action_controller.request.path_parameters']
  end

  config.store_global_info do |storage|
    storage[:global_info][:app_name] = Rails.application.class.parent_name
    storage[:global_info][:created_at] = Time.now
  end

and tried to add

config.after_intitalize do
  ErrorMessage.send(:store, :request_info)
  ErrorMessage.send(:store, :exception_info)
  ErrorMessage.send(:store, :environment_info)
  ErrorMessage.send(:store, :global_info)
end

to get the different informations at the server side as four hashes, but this didn't worked out with

NoMethodError: undefined method after_intitalize' for #<RailsExceptionHandler::Configuration`

Could you give me a hint on how to setup your new release? Would be very happy to test this out.

Thanks & cheers.

@davidjrice
Copy link
Contributor

Tried this as well. Works fine for me storing via active_record. I was also able to easily add an extra field I wanted to store in the db and in the config and it worked great.

However, having an issue with the post to remote url storage strategy.

undefined method `has_key?' for "[:user_info, nil]":String

It would appear that the error_message params are coming through as a string.

@bjorntrondsen
Copy link
Owner

nilsrchtr: There's a spelling error: "after_intitalize" is supposed to be "after_initialize". That's my fault, I made the spelling error in my post when explaining how to do it.
The nested hash syntax is also slightly wrong because we have to initalize the keys before nesting:

storage[:request_info] = {}
storage[:request_info][:target_url] = request.url

Or do

storage[:request_info] = {
  :target_url => request.url
}

David, you're getting that error message on the server side, right? ( The app that receives the error messages via http post)
I will test the remote_url strategy and get back to you.

@bjorntrondsen
Copy link
Owner

David: You were right, the params did come through as a string, not a hash. It turns out that Net::HTTP::Post doesn't handle nested hashes. I have updated the remote_url storage strategy to handle nested hashes manually. Could you try the latest version in master?

@davidjrice
Copy link
Contributor

Can confirm remote URL strategy is working again!

@bjorntrondsen
Copy link
Owner

Did you get a chance to test this yet, Niels?
I'm very swamped with work right now, but hopefully I will find some time within the next couple of weeks to write up the documentation and release v2.0

@nlsrchtr
Copy link
Author

I'm currently on vacations. I'll be back in the beginning of may and would really like to test it out!

@bjorntrondsen
Copy link
Owner

Version 2 has been released with these changes. Open a new ticket if you have any problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants