Logging and swallowing of exceptions causes false-postitive test results

[stevejalim]

https://github.com/yunojuno/django-side-effects/blob/0c1a01f029a4862babcb0820b02ea3b59a9407fc/side_effects/registry.py#L94 swallows all exceptions and sends them to the logger as an exception().

This is arguably OK in production (though am not a fan, personally), insomuch as a site with properly configured logging will catch this and inform the maintainers who will/should fix it ASAP (and side effects may be deemed acceptable to lose).

However, unit tests don't benefit from this, and so will still register a pass, even though the side effect for a decorated method or function is failing -- denying developers who aren't reading their test run output closely the chance to fix it before it hits production. I've seen this happen.

As an example, here's some output from a a test which uses a side-effect to send an email, which fails because the template is missing, but which passes in test terms, because the exception is caught.

test_submit_something_to_someone (myproj.tests.apps.myapp.test_models.USFreelancerExternalComplianceStateChangingTests) ... ERROR    Error running side_effect function 'myproj.apps.myapp.side_effects.email.submit_something_to_someone'
Traceback (most recent call last):
  File "/home/python_user/.pyenv/versions/3.6.1/lib/python3.6/site-packages/side_effects/registry.py", line 92, in _run_func
    func(*args, **kwargs)
  File "/app/myproj/apps/myapp/side_effects/email.py", line 34, in submit_something_to_someone
    context={}
  File "/app/myproj/apps/messaging/email2.py", line 44, in send_mail
    email_queue.enqueue(_send_mail, template_name, context, **kwargs)
  File "/home/python_user/.pyenv/versions/3.6.1/lib/python3.6/site-packages/rq/queue.py", line 280, in enqueue
    job_id=job_id, at_front=at_front, meta=meta)
  File "/home/python_user/.pyenv/versions/3.6.1/lib/python3.6/site-packages/django_rq/queues.py", line 60, in enqueue_call
    return self.original_enqueue_call(*args, **kwargs)
  File "/home/python_user/.pyenv/versions/3.6.1/lib/python3.6/site-packages/django_rq/queues.py", line 56, in original_enqueue_call
    return super(DjangoRQ, self).enqueue_call(*args, **kwargs)
  File "/home/python_user/.pyenv/versions/3.6.1/lib/python3.6/site-packages/rq/queue.py", line 232, in enqueue_call
    job = self.enqueue_job(job, at_front=at_front)
  File "/home/python_user/.pyenv/versions/3.6.1/lib/python3.6/site-packages/rq/queue.py", line 307, in enqueue_job
    job = self.run_job(job)
  File "/home/python_user/.pyenv/versions/3.6.1/lib/python3.6/site-packages/rq/queue.py", line 237, in run_job
    job.perform()
  File "/home/python_user/.pyenv/versions/3.6.1/lib/python3.6/site-packages/rq/job.py", line 560, in perform
    self._result = self._execute()
  File "/home/python_user/.pyenv/versions/3.6.1/lib/python3.6/site-packages/rq/job.py", line 566, in _execute
    return self.func(*self.args, **self.kwargs)
  File "/app/myproj/apps/messaging/email2.py", line 62, in _send_mail
    email = template.create_message(context, **kwargs)
AttributeError: 'NoneType' object has no attribute 'create_message'
ok

So, because that AttributeError is swallowed even though it's complaining about something that effectively breaks production, I'd call that a false positive.

Any chance we can have a settings.SIDE_EFFECTS_RAISE_EXCEPTIONS config option or similar, so that tests (and production if you want it) can be fail hard?

[hugorodgerbrown]

Happy to add a flag (which by preference I would bind hard to DEBUG) to blow up when an exception occurs in the side-effect, however, ignoring exceptions in the side-effects is a fundamental part of the framework. If the side-effect failing halts the running of the source function, then arguably it's not a side-effect.

A good example of this is the password reset process. In this specific case the email isn't a side-effect of the reset process, it is the reset process - and the function should fail hard if there is a problem. In this case it shouldn't be run through the side-effects framework.

(NB one other consideration is that one of the core tenets of side-effects (as defined in this projects) is that they can be run async - which means catching exceptions isn't possible.)

[hugorodgerbrown]

Fixed - there is now a SIDE_EFFECTS_SUPPRESS_ERRORS setting / env var that you can use to override the behaviour.

from side_effects import settings

def test_func(self):
    settings.SUPPRESS_ERRORS = False
    self.assertRaises(Exception, do_function_with_bad_side_effect)

[hugorodgerbrown]

Released to PyPI as v0.2.1