Repository navigation
feat(taskworker): Restore autoreload for the local taskworker - #128673
Conversation
When you change backend code while running Sentry locally, the web server picks up the change on its own, but the taskworker doesn't. You have to stop and restart the worker by hand, which is easy to forget, and then tasks quietly run the old code. It's also why people end up running a separate worker with `devservices foreground taskworker` instead of the one the devserver starts. The worker used to have an autoreload option, but it was removed in #96486. That option used Django's reloader, which runs your code on a background thread and keeps the main thread for watching files. Once the taskworker started setting up its own handlers for shutdown signals, it broke, because Python only lets the main thread do that. The worker crashed on startup, so the option was taken out. This brings `--autoreload` back with a different setup. A small parent process watches the Python files and does nothing else. The worker runs as its own separate process, so it has its main thread again and can set up its signal handlers like normal. When a file changes, the parent asks the worker to shut down the usual way, waits for it to finish, and starts a fresh one. The devserver now passes the flag to its worker whenever reload is on (the default), and the devservices taskworker programs use it too. Production doesn't pass the flag, so nothing changes there. Saving several times in quick succession needed one more piece. A save could land while the new worker was still starting up, before it had set up its signal handlers, and the stop request then printed a KeyboardInterrupt traceback. Now the parent waits for the new worker to say it's done starting before it restarts it again. Saves made during that wait still cause one more restart afterwards, so nothing is missed. If the worker crashes while starting, say from a typo, the parent notices it's gone and stops waiting, so the next save brings it back. The file watching comes from `watchfiles`, which is already installed because our web server's reload uses it. getsentry needs a matching one-line config change, which should land after this. ai slop diagrams: before: <img width="645" height="273" alt="image" src="https://github.com/user-attachments/assets/c939f6bc-5ef1-4618-bb8a-4fd5dfd90a9d" /> after: <img width="634" height="304" alt="image" src="https://github.com/user-attachments/assets/be1c4bf9-24bc-4daf-952b-ba33c5e06340" />
watchfiles imports anyio, which tries to import sniffio. test_devimports blocks sniffio with a plain ImportError, which anyio doesn't catch, so importing watchfiles at CLI startup broke test_startup_imports[sentry]. Import it inside the autoreload path instead.
The reloaded worker reported ready right after configure(), but it only registers its SIGINT handler later in start(). During a burst of saves the restart SIGINT landed in that gap, so the worker skipped its shutdown, hung waiting on its children, and was SIGKILLed by watchfiles, leaving the children orphaned. Report ready once the handler is in place instead.
Stopping the autoreloading worker while a new one was still starting printed KeyboardInterrupt tracebacks and made the parent exit 1. Catch the interrupt around configure() as well as the worker run, ignore the second SIGINT watchfiles sends during that exit, and treat an interrupt while waiting on a restart as a normal stop.
|
bugbot review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8c77d79. Configure here.
| except KeyboardInterrupt: | ||
| # Stopped while waiting on a restart. watchfiles has already stopped | ||
| # the worker on its way out. | ||
| pass |
There was a problem hiding this comment.
SIGTERM orphans the reloaded worker
Medium Severity
The autoreload parent only catches KeyboardInterrupt and never handles SIGTERM. Honcho and other supervisors stop sentry run taskworker by signaling that PID, so the parent exits immediately and the spawned worker plus its children keep running.
Reviewed by Cursor Bugbot for commit 8c77d79. Configure here.
There was a problem hiding this comment.
This one is a false positive. watchfiles.run_process calls catch_sigterm() before it starts the worker, which turns SIGTERM into a KeyboardInterrupt. Its finally: process.stop() then stops the worker before our except runs. Honcho also signals the whole process group with os.killpg, not just this PID. I tested SIGTERM to the parent PID alone, including in the middle of a restart, and it exits 0 with nothing left running.
|
^ running all tests just to make sure |


This re-lands #128564, which was reverted in 3e7ab78 because it broke a test on master.
That test makes sure starting the Sentry command line doesn't import any package that only dev installs have. The original PR imported the file watching library at the top of the file. That library pulls in another one, which tries to load an optional package that only dev installs have. Production doesn't have that package, and the library handles it being missing, so prod was fine. But the test blocks it with a slightly different error than the library expects, so the test failed. Selective testing didn't pick that test on the original PR, which is why its CI was green.
The fix is to import the file watcher only when the worker is started with autoreload, so normal startup never loads it. While re-testing, saving a file several times a second also turned up a bug in the original. The worker told the parent it was ready to be restarted a bit too early, before it had set up its handler for the stop request. During a burst of saves the stop request landed in that gap, so the worker skipped its normal shutdown, hung, and got force-killed, leaving its child processes running forever. Now it only says it's ready once that handler is in place. Stopping the worker while a new one was still starting also printed tracebacks and exited with an error, so that's quiet now too.
I stress tested it with saves 20 times a second, about 11,000 saves a second for 5 seconds, five files at once, bursts with random pauses, random gaps for 90 seconds, and a module that keeps breaking and getting fixed. I also stopped it mid-burst with Ctrl-C and with the devserver's shutdown signal. There was never more than one worker, nothing got orphaned or force-killed, and it always ended up running the latest code. Rapid saves get batched, so 200 saves turn into about 4 restarts.
Everything else is the same as the original PR, which is below.
When you change backend code while running Sentry locally, the web server picks up the change on its own, but the taskworker doesn't. You have to stop and restart the worker by hand, which is easy to forget, and then tasks quietly run the old code. It's also why people end up running a separate worker with
devservices foreground taskworkerinstead of the one the devserver starts.The worker used to have an autoreload option, but it was removed in #96486. That option used Django's reloader, which runs your code on a background thread and keeps the main thread for watching files. Once the taskworker started setting up its own handlers for shutdown signals, it broke, because Python only lets the main thread do that. The worker crashed on startup, so the option was taken out.
This brings
--autoreloadback with a different setup. A small parent process watches the Python files and does nothing else. The worker runs as its own separate process, so it has its main thread again and can set up its signal handlers like normal. When a file changes, the parent asks the worker to shut down the usual way, waits for it to finish, and starts a fresh one. The devserver now passes the flag to its worker whenever reload is on (the default), and the devservices taskworker programs use it too. Production doesn't pass the flag, so nothing changes there.Saving several times in quick succession needed one more piece. A save could land while the new worker was still starting up, before it had set up its signal handlers, and the stop request then printed a KeyboardInterrupt traceback. Now the parent waits for the new worker to say it's done starting before it restarts it again. Saves made during that wait still cause one more restart afterwards, so nothing is missed. If the worker crashes while starting, say from a typo, the parent notices it's gone and stops waiting, so the next save brings it back.
The file watching comes from
watchfiles, which is already installed because our web server's reload uses it.getsentry needs a matching one-line config change, which should land after this.
ai slop diagrams:
before:

after:
