Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that the exception dictionaries are not mutated when generating a html report #1784

Merged
merged 2 commits into from
Jun 11, 2021

Conversation

mboutet
Copy link
Contributor

@mboutet mboutet commented Jun 9, 2021

This PR fixes the following issue:

[2021-06-09 20:36:13,702] master1/CRITICAL/locust.runners: Unhandled exception in greenlet: <Greenlet at 0x7ff41a2846a0: <bound method MasterRunner.client_listener of <locust.runners.MasterRunner object at 0x7ff3ea9d4b80>>>
Traceback (most recent call last):
  File "src/gevent/greenlet.py", line 906, in gevent._gevent_cgreenlet.Greenlet.run
  File "/home/adminuser/.venv/lib/python3.8/site-packages/locust/runners.py", line 842, in client_listener
    self.log_exception(msg.node_id, msg.data["msg"], msg.data["traceback"])
  File "/home/adminuser/.venv/lib/python3.8/site-packages/locust/runners.py", line 422, in log_exception
    row["nodes"].add(node_id)
AttributeError: 'str' object has no attribute 'add'

If a user generated a html report during a load test run, then the nodes field in every exception dict was mutated to a string.

This also caused the following when hovering on one of the exception in the web UI:

Screen Shot 2021-06-09 at 18 08 27

@mboutet
Copy link
Contributor Author

mboutet commented Jun 9, 2021

PS: I will incorporate this fix in the distribution branch because it is causing my load tests to crash (I use the html report feature quite a lot).

locust/html.py Outdated
@@ -36,7 +38,8 @@ def get_html_report(environment, show_download_link=True):
requests_statistics = list(chain(sort_stats(stats.entries), [stats.total]))
failures_statistics = sort_stats(stats.errors)
exceptions_statistics = []
for exc in environment.runner.exceptions.values():
exceptions = deepcopy(environment.runner.exceptions)
for exc in exceptions.values():
exc["nodes"] = ", ".join(exc["nodes"])
exceptions_statistics.append(exc)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just do something like exceptions_statistics.append(", ".join(exc["nodes"])) instead? No need to copy runner.exceptions if exceptions is never used again, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exceptions_statistics must also contain the other fields of the exception dictionary as the html template also needs the other properties:

locust/locust/html.py

Lines 66 to 80 in 60cde73

res = render_template(
"report.html",
int=int,
round=round,
requests_statistics=requests_statistics,
failures_statistics=failures_statistics,
exceptions_statistics=exceptions_statistics,
start_time=start_time,
end_time=end_time,
host=host,
history=history,
static_js="\n".join(static_js),
static_css="\n".join(static_css),
show_download_link=show_download_link,
)

If you prefer, it could be something like this instead of the for-loop:

exceptions_statistics = [
    {**exc, "nodes": ", ".join(exc["nodes"])}
    for exc in environment.runner.exceptions.values()
]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it. Looks better now in my opinion.

@cyberw
Copy link
Collaborator

cyberw commented Jun 11, 2021

Thx!

@cyberw cyberw merged commit 198fc7c into locustio:master Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants