concerto / bandshell

A Local Management Utility for the Concerto Player
Other
7 stars 7 forks source link

Network connection #29

Open gbprz opened 10 years ago

gbprz commented 10 years ago

While testing a screen at RPI, one of our screens lost it's network connection. Rather than gracefully crashing or returning once a connection was regained, the screen crashed and left this error in the bandshell logs:

bandshelld.output

Timeout::Error - Timeout::Error:
    /usr/lib/ruby/1.9.1/net/protocol.rb:146:in `rescue in rbuf_fill'
    /usr/lib/ruby/1.9.1/net/protocol.rb:140:in `rbuf_fill'
    /usr/lib/ruby/1.9.1/net/protocol.rb:122:in `readuntil'
    /usr/lib/ruby/1.9.1/net/protocol.rb:132:in `readline'
    /usr/lib/ruby/1.9.1/net/http.rb:2562:in `read_status_line'
    /usr/lib/ruby/1.9.1/net/http.rb:2551:in `read_new'
    /usr/lib/ruby/1.9.1/net/http.rb:1319:in `block in transport_request'
    /usr/lib/ruby/1.9.1/net/http.rb:1316:in `catch'
    /usr/lib/ruby/1.9.1/net/http.rb:1316:in `transport_request'
    /usr/lib/ruby/1.9.1/net/http.rb:1293:in `request'
    /var/lib/gems/1.9.1/gems/bandshell-0.9/lib/bandshell/hardware_api.rb:105:in `block in get_with_auth'
    /usr/lib/ruby/1.9.1/net/http.rb:745:in `start'
    /usr/lib/ruby/1.9.1/net/http.rb:557:in `start'
    /var/lib/gems/1.9.1/gems/bandshell-0.9/lib/bandshell/hardware_api.rb:104:in `get_with_auth'
    /var/lib/gems/1.9.1/gems/bandshell-0.9/lib/bandshell/hardware_api.rb:161:in `get_player_info'
    /var/lib/gems/1.9.1/gems/bandshell-0.9/lib/bandshell/application/app.rb:414:in `update_player_info'
    /var/lib/gems/1.9.1/gems/bandshell-0.9/lib/bandshell/application/app.rb:395:in `block in <class:ConcertoConfigServer>'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1603:in `call'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1603:in `block in compile!'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:966:in `[]'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:966:in `block (3 levels) in route!'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:985:in `route_eval'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:966:in `block (2 levels) in route!'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1006:in `block in process_route'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1004:in `catch'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1004:in `process_route'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:964:in `block in route!'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:963:in `each'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:963:in `route!'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1076:in `block in dispatch!'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1058:in `block in invoke'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1058:in `catch'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1058:in `invoke'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1073:in `dispatch!'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:898:in `block in call!'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1058:in `block in invoke'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1058:in `catch'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1058:in `invoke'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:898:in `call!'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:886:in `call'
    /var/lib/gems/1.9.1/gems/rack-protection-1.5.3/lib/rack/protection/xss_header.rb:18:in `call'
    /var/lib/gems/1.9.1/gems/rack-protection-1.5.3/lib/rack/protection/path_traversal.rb:16:in `call'
    /var/lib/gems/1.9.1/gems/rack-protection-1.5.3/lib/rack/protection/json_csrf.rb:18:in `call'
    /var/lib/gems/1.9.1/gems/rack-protection-1.5.3/lib/rack/protection/base.rb:49:in `call'
    /var/lib/gems/1.9.1/gems/rack-protection-1.5.3/lib/rack/protection/base.rb:49:in `call'
    /var/lib/gems/1.9.1/gems/rack-protection-1.5.3/lib/rack/protection/frame_options.rb:31:in `call'
    /var/lib/gems/1.9.1/gems/rack-1.5.2/lib/rack/nulllogger.rb:9:in `call'
    /var/lib/gems/1.9.1/gems/rack-1.5.2/lib/rack/head.rb:11:in `call'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:180:in `call'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:2014:in `call'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1478:in `block in call'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1788:in `synchronize'
    /var/lib/gems/1.9.1/gems/sinatra-1.4.5/lib/sinatra/base.rb:1478:in `call'
    /var/lib/gems/1.9.1/gems/rack-1.5.2/lib/rack/handler/webrick.rb:60:in `service'
    /usr/lib/ruby/1.9.1/webrick/httpserver.rb:138:in `service'
    /usr/lib/ruby/1.9.1/webrick/httpserver.rb:94:in `run'
    /usr/lib/ruby/1.9.1/webrick/server.rb:191:in `block in start_thread'

bandshell-timer.rb.output

/usr/lib/ruby/1.9.1/net/protocol.rb:146:in `rescue in rbuf_fill': Timeout::Error (Timeout::Error)
    from /usr/lib/ruby/1.9.1/net/protocol.rb:140:in `rbuf_fill'
    from /usr/lib/ruby/1.9.1/net/protocol.rb:122:in `readuntil'
    from /usr/lib/ruby/1.9.1/net/protocol.rb:132:in `readline'
    from /usr/lib/ruby/1.9.1/net/http.rb:2562:in `read_status_line'
    from /usr/lib/ruby/1.9.1/net/http.rb:2551:in `read_new'
    from /usr/lib/ruby/1.9.1/net/http.rb:1319:in `block in transport_request'
    from /usr/lib/ruby/1.9.1/net/http.rb:1316:in `catch'
    from /usr/lib/ruby/1.9.1/net/http.rb:1316:in `transport_request'
    from /usr/lib/ruby/1.9.1/net/http.rb:1293:in `request'
    from /usr/lib/ruby/1.9.1/net/http.rb:1195:in `request_get'
    from /usr/lib/ruby/1.9.1/net/http.rb:455:in `block in get_response'
    from /usr/lib/ruby/1.9.1/net/http.rb:745:in `start'
    from /usr/lib/ruby/1.9.1/net/http.rb:454:in `get_response'
    from /var/lib/gems/1.9.1/gems/bandshell-0.9/bin/bandshell-timer.rb:28:in `block in <top (required)>'
    from /var/lib/gems/1.9.1/gems/bandshell-0.9/bin/bandshell-timer.rb:25:in `loop'
    from /var/lib/gems/1.9.1/gems/bandshell-0.9/bin/bandshell-timer.rb:25:in `<top (required)>'
    from /var/lib/gems/1.9.1/gems/daemons-1.1.9/lib/daemons/application.rb:203:in `load'
    from /var/lib/gems/1.9.1/gems/daemons-1.1.9/lib/daemons/application.rb:203:in `start_load'
    from /var/lib/gems/1.9.1/gems/daemons-1.1.9/lib/daemons/application.rb:298:in `start'
    from /var/lib/gems/1.9.1/gems/daemons-1.1.9/lib/daemons/controller.rb:70:in `run'
    from /var/lib/gems/1.9.1/gems/daemons-1.1.9/lib/daemons.rb:147:in `block in run'
    from /var/lib/gems/1.9.1/gems/daemons-1.1.9/lib/daemons/cmdline.rb:109:in `call'
    from /var/lib/gems/1.9.1/gems/daemons-1.1.9/lib/daemons/cmdline.rb:109:in `catch_exceptions'
    from /var/lib/gems/1.9.1/gems/daemons-1.1.9/lib/daemons.rb:146:in `run'
    from /var/lib/gems/1.9.1/gems/bandshell-0.9/bin/bandshelld:21:in `<top (required)>'
    from /usr/local/bin/bandshelld:23:in `load'
    from /usr/local/bin/bandshelld:23:in `<main>'
kench commented 10 years ago

We're not catching the Timeout::Error, so it's bubbling all the way back up.

https://github.com/concerto/bandshell/blob/master/bin/bandshell-timer.rb

kench commented 10 years ago

I suggest having a catch-all rescue block as shown here:

http://coderrr.wordpress.com/2008/11/07/the-simple-way-to-print-exceptions-in-ruby/

tyrantkhan commented 10 years ago

a rescue all is a very poor paradigm to follow. I'm pretty sure Yukihiro Matsumoto will stab us if we do that.

On Wed, Aug 13, 2014 at 1:34 AM, Kenley Cheung notifications@github.com wrote:

I suggest having a catch-all rescue block as shown here:

http://coderrr.wordpress.com/2008/11/07/the-simple-way-to-print-exceptions-in-ruby/

— Reply to this email directly or view it on GitHub https://github.com/concerto/bandshell/issues/29#issuecomment-52011565.

augustf commented 10 years ago

Yeah-I mean we're only using rescue here because there's not another clean choice. Every time you hit a rescue, you incur a massive performance hit in processing the exception; they're to be avoided at most any cost.

augustf commented 10 years ago

Upon further thought though, @khanh, if catching any exception is essential to continued operation of the screen, shouldn't we pursue that route whatever the style consequences?

kench commented 10 years ago

@augustf, @khanh - As ugly as my suggestion is, keeping the screen running was the intent. We need to fail gracefully in case of a network failure.

tyrantkhan commented 10 years ago

that's fine, we can rescue Timeout::Error , but we shouldn't get in the lazy habit of catching all errors. my bestie yuki would be sad.. like a gray snowy day.

On Wed, Aug 13, 2014 at 9:38 PM, Kenley Cheung notifications@github.com wrote:

@augustf https://github.com/augustf, @khanh https://github.com/khanh

  • As ugly as my suggestion is, keeping the screen running was the intent. We need to fail gracefully in case of a network failure.

— Reply to this email directly or view it on GitHub https://github.com/concerto/bandshell/issues/29#issuecomment-52134248.

kench commented 10 years ago

On a bigger note, we're not the only parties facing this problem:

http://stackoverflow.com/questions/5370697/what-s-the-best-way-to-handle-exceptions-from-nethttp

It's been suggested to use an external HTTP library instead.

mikldt commented 10 years ago

httparty or something similar sounds like a good idea.

Right now I'm working on pulling out all the API requests into a separate class so the code is a little less convoluted. Once I get that in, we should definitely try to implement some form of error handling here.