Summary

Gracefully handle errors communicating with the queue.

Review Request #14374 — Created March 18, 2025 and updated March 24, 2025, 9:10 a.m.

Information

Owner

chipx86

Repository

ReviewBot

Branch

release-4.x

Bugs

Depends On

Reviewers

Groups

reviewbot

People

Description

When the queue is down or non-responsive, we end up with long exception
traces in the log files but nothing useful on the front-end. Tool
updates and tool runs will spin and spin until they time out.

We now have error handling code around all code communicating over
Celery, logging errors and stack traces with a trace ID.

Tool runs (both manual and automatic) will fail to the error state with
an "error running tool (error ID XYZ)", instead of waiting to time out.

Worker status checks will once again display a suitable error message
and include information in the logs. We had code that attempted to
provide a good result, but it was checking for IOError, and nothing in
the Celery communication code uses that anymore.

Tool refreshes in the Tools database list now alert with an error and
reset the state. This has also been updated to be more accessible and
prioritize Ink styling if available.

Testing Done

Shut down RabbitMQ and tested all the functionality: Manual tool runs,
automatic tool runs, worker status checks, and tool refreshes. Verified
the log output and the user-facing output in all cases.

Tested the newer tool refresh UI on Review Board 6 and 7.

Unit tests pass.

Commits

Summary	ID
Gracefully handle errors communicating with the queue. When the queue is down or non-responsive, we end up with long exception traces in the log files but nothing useful on the front-end. Tool updates and tool runs will spin and spin until they time out. We now have error handling code around all code communicating over Celery, logging errors and stack traces with a trace ID. Tool runs (both manual and automatic) will fail to the error state with an "error running tool (error ID XYZ)", instead of waiting to time out. Worker status checks will once again display a suitable error message and include information in the logs. We had code that attempted to provide a good result, but it was checking for `IOError`, and nothing in the Celery communication code uses that anymore. Tool refreshes in the Tools database list now alert with an error and reset the state. This has also been updated to be more accessible and prioritize Ink styling if available.	8bdcbf673fac75368ecdcec9b3fe84255b869a47

Summary

Gracefully handle errors communicating with the queue.

When the queue is down or non-responsive, we end up with long exception traces in the log files but nothing useful on the front-end. Tool updates and tool runs will spin and spin until they time out. We now have error handling code around all code communicating over Celery, logging errors and stack traces with a trace ID. Tool runs (both manual and automatic) will fail to the error state with an "error running tool (error ID XYZ)", instead of waiting to time out. Worker status checks will once again display a suitable error message and include information in the logs. We had code that attempted to provide a good result, but it was checking for `IOError`, and nothing in the Celery communication code uses that anymore. Tool refreshes in the Tools database list now alert with an error and reset the state. This has also been updated to be more accessible and prioritize Ink styling if available.

8bdcbf673fac75368ecdcec9b3fe84255b869a47

Files

Issues

Description	From	Last Updated
I'm pretty sure that you can just use ink-c-button here. All <button> elements already receive rb-c-button styling without having to …	maubin	March 21, 2025, 4:04 p.m.
Is the indentation here supposed to be outside of the {% or inside? I thought it was supposed to be …	maubin	March 24, 2025, 9:10 a.m.

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Fix it!

extension/reviewbotext/templates/admin/reviewbotext/tool/change_list.html (Diff revision 1)

An issue was opened. Show all issues

I'm pretty sure that you can just use ink-c-button here. All <button> elements already receive rb-c-button styling without having to add the class name (this happens in reviewboard/static/rb/css/ui/buttons.less). And it's safe to have ink-c-button classes on RB6 servers, since it doesn't exist and won't do anything. I've been doing this for some buttons in Power Pack 6 and it works.

extension/reviewbotext/templates/admin/reviewbotext/tool/change_list.html (Diff revision 1)

The issue has been resolved. Show all issues

Is the indentation here supposed to be outside of the {% or inside? I thought it was supposed to be inside like:
{% if ... %}
...
{%  trans ... %}

david March 23, 2025, 9 p.m.

We do indentation inside for block tags. {% trans %} is an inline tag, so we just do it like a regular part of the HTML.
So:

{% if ... %}
{%  blocktrans %}
 ...
{%  endblocktrans %}
{% endif %}

{% if ... %}
 {% trans "..." %}
{% endif %}

maubin March 24, 2025, 9:10 a.m.
```
Oh got it, thanks for clarifying.
```