openedx / openedx/openedx-events

Event handler errors are suppressed by default in tests

Open
#569 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
13
Forks
33
Avg merge
14h 37m
Merged PRs (30d)
4

Description

When I try using an event in my openedx-platform tests, I found that throwing an exception in an event handler has no effect, and the exception is silently ignored.

This is because openedx-events uses send_robust by default even in tests, which suppresses exceptions thrown in the event receivers. This seems to go against the original intention of https://github.com/openedx/openedx-events/pull/29 which said:

This PR changes which send method to use when sending an event, so they don't fail during runtime. Instead, they will be allowed to fail in a testing/dev environment.

I found that I can work around this by adding EventsIsolationMixin to my test class, and adding this setup code:

    @classmethod
    def setUpClass(cls):
        """Test setup"""
        super().setUpClass()
        # By default, errors thrown in signal handlers get suppressed. We want to see them though!
        cls.allow_send_events_failure(*(s.event_type for s in OpenEdxPublicSignal.all_events()))

But I think that this shouldn't be necessary - suppressing exceptions during tests is confusing and unexpected behavior that takes a while to debug. Also, the code shown above "leaks" because it affects any other tests that will run afterward. The events API provides a way to "allow send events failure" but it doesn't provide a way to undo that.

I recommend that you just scrap all of that and add a setting called SUPPRESS_EVENT_EXCEPTIONS or EVENT_SENDING_ROBUST_BY_DEFAULT which is True in envs/production.py and otherwise defaults to False.

Other issues
  1. "allow send events failure" is very ambiguous. Does that mean "allow events to fail, by ignoring their exceptions", or does that mean "allow exceptions (failures) to be reported" ? Calling this "suppress exceptions" would be way more clear.

  2. I'm not the only one confused. The documentation of OpenEdxPublicSignal.allow_send_event_failure says:

https://github.com/openedx/openedx-events/blob/34d3fce371f14c892d77284f9fcba8a5e67fc544/openedx_events/tooling.py#L267-L273

But this is the opposite of what it actually does!

https://github.com/openedx/openedx-events/blob/34d3fce371f14c892d77284f9fcba8a5e67fc544/openedx_events/tooling.py#L168-L171

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading openedx_events/tooling.py at lines 168-171 and 267-273, then inspect envs/production.py to understand the current event-sending behavior and configuration. Clarify how exceptions should behave in tests versus production, how the setting should be named, and how the misleading allow_send_event_failure documentation should change. Done means the behavior is consistent with the documented configuration and does not leak between tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend-api-design, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.