--run-time flag is not respected if there is an exception in a test_stop listener

[camilojimenez]

Describe the bug

On a headless session, if there is an exception thrown from a test_stop listener, the run-time flag is not respected and the
session keeps running indefinitely.

Expected behavior

The session should stop even if the listeners throw exceptions.

Actual behavior

The traceback is printed, and the session keeps running.

Steps to reproduce

Take, for example, the following locustfile.py:

from locust import HttpUser, events, task
import time

@events.test_stop.add_listener
def on_test_stop(**kwargs):
  1/0

class NOOP(HttpUser):
  @task
  def index_page(self):
    time.sleep(10)

If locust is run with the following command line:

locust -t1 -r1 -u10 --headless --host http://www.google.com

you see the exception being raised and the session continues:

[2020-07-03 03:37:18,086] instance-1/INFO/locust.main: Time limit reached. Stopping Locust.
Traceback (most recent call last):
  File "src/gevent/greenlet.py", line 854, in gevent._gevent_cgreenlet.Greenlet.run
  File "/home/camilojimenez/locust/venv/lib/python3.7/site-packages/locust/main.py", line 231, in timelimit_stop
    runner.quit()
  File "/home/camilojimenez/locust/venv/lib/python3.7/site-packages/locust/runners.py", line 294, in quit
    self.stop()
  File "/home/camilojimenez/locust/venv/lib/python3.7/site-packages/locust/runners.py", line 340, in stop
    self.environment.events.test_stop.fire(environment=self.environment)
  File "/home/camilojimenez/locust/venv/lib/python3.7/site-packages/locust/event.py", line 33, in fire
    handler(**kwargs)
  File "/home/camilojimenez/locust/locustfile.py", line 6, in on_test_stop
    1/0
ZeroDivisionError: division by zero
2020-07-03T03:37:18Z <Greenlet at 0x7fcad134b9d8: timelimit_stop> failed with ZeroDivisionError

[2020-07-03 03:37:18,091] instance-1/CRITICAL/locust.main: Unhandled exception in greenlet: <Greenlet at 0x7fcad134b9d8: timelimit_stop>
Traceback (most recent call last):
  File "src/gevent/greenlet.py", line 854, in gevent._gevent_cgreenlet.Greenlet.run
  File "/home/camilojimenez/locust/venv/lib/python3.7/site-packages/locust/main.py", line 231, in timelimit_stop
    runner.quit()
  File "/home/camilojimenez/locust/venv/lib/python3.7/site-packages/locust/runners.py", line 294, in quit
    self.stop()
  File "/home/camilojimenez/locust/venv/lib/python3.7/site-packages/locust/runners.py", line 340, in stop
    self.environment.events.test_stop.fire(environment=self.environment)
  File "/home/camilojimenez/locust/venv/lib/python3.7/site-packages/locust/event.py", line 33, in fire
    handler(**kwargs)
  File "/home/camilojimenez/locust/locustfile.py", line 6, in on_test_stop
    1/0
ZeroDivisionError: division by zero
 Name                                                          # reqs      # fails     Avg     Min     Max  |  Median   req/s failures/s
--------------------------------------------------------------------------------------------------------------------------------------------
--------------------------------------------------------------------------------------------------------------------------------------------
 Aggregated                                                         0     0(0.00%)       0       0       0  |       0    0.00    0.00

Environment

• OS: MacOS
• Python version: 3.7.3
• Locust version: 1.1

[camilojimenez]

I added some simple exception handling to fix my issue in #1462, but I am not sure if that's the preferred way of handling this (for example, there is no logging of the exception)

[cyberw]

Awesome, thanks!
Maybe replace

...
except:
    pass

with

import logging
...
except Exception as e:
    logging.error("Uncaught exception in event handler: %s", e)

[camilojimenez]

Thanks @cyberw!, I saw that the test coverage was reduced by this change, in which file do you recommend I write a test for this?

[cyberw]

👍 I think it belongs in a completely new file called test_event.py.

[cyberw]

Actually, changed my mind, put it in test_runners.py. The other test_stop/start tests are there, and I'm a fan of testing slightly bigger things (runner+event instead of just event) when possible.

[cyberw]

Oh, and one more thing: I think an exception in one of the event methods should set the return code of locust to fail.

Import and set log.unhandled_greenlet_exception to True, it is close enough to what has happened, and will be checked when Locust is shutting down.

[camilojimenez]

neat! thanks for all the help, I just pushed new changes, please let me know if you have any other suggestions.

[cyberw]

LGTM :) Thanks!