Rescue from ScriptError

[sharshenov]

I found that sneakers worker becomes frozen after raised NotImplementedError.
Code to reproduce:

class MyWorker
  include Sneakers::Worker

  from_queue  'lolwut'

  def work(msg)
    raise NotImplementedError
    ack!
  end

This PR fixes it.

[gabrieljoelc]

Good catch! I made one suggestion to make the test more explicit

[kalys]

I think this PR shouldn't be merged. If you have error in your code then you have to fix it in your code.

[sharshenov]

Background processing should be reliable. It should not become frozen if coder writes bad code leading to error.

[gabrieljoelc]

@kalys thanks for weighing in.

Errors have always been rescued in workers and passed to the Sneakers::Handlers class of the user's choosing (see these lines). This is so user's can decide how to handle unhandled exceptions. This PR was just for including an additional Ruby base Error class so that the framework would pass ScriptError's as well as StandardError's to the Sneakers::Handlers class so that the framework wouldn't end up in an unknown state (e.g. "frozen").

[kalys]

Background processing should be reliable. It should not become frozen if coder writes bad code leading to error.

Coder should cover his/her code with tests to detect ScriptErrors and not use system layer NotImplementedError in application layer.

@gabrieljoelc
https://ruby-doc.org/core-2.5.0/NotImplementedError.html

Raised when a feature is not implemented on the current platform. For example, methods depending on the fsync or fork system calls may raise this exception if the underlying operating system or Ruby runtime does not support them.

It's not ok to catch error when some feature is unavailable in current ruby implementation or OS.

http://chrisstump.online/2016/03/23/stop-abusing-notimplementederror/

[gabrieljoelc]

We catch the base class here not NotImplementedError, so this could allow maintaining Sneakers error handling behavior for other sub-types of ScriptError as well (i.e. LoadError). Bottom line: we want want Sneakers to pass unhandled exceptions to worker_error instead of freezing.

NotImplementedError usage discussion is for another forum, please.

[sharshenov]

@gabrieljoelc Unfortunately, the problem of stale workers is still actual. Some gems introduce their own exceptions not based on StandardError and SystemStackError also leads to frozen state of consumer. I've made another attempt to fix the problem by introducing ensure. #441