Catch OSError when persisting heartbeat to queue#113
Conversation
Prevents a crash when the disk is full by catching OSError in add_request() and logging a warning instead of letting it propagate. Fixes ActivityWatch#8
Greptile SummaryThis PR adds graceful error handling to
Confidence Score: 4/5Safe to merge — the core fix is a small, well-scoped try/except with a companion test; only style nits remain. The logic change is minimal and correct: an uncaught OSError from a disk-full scenario now becomes a logged warning. The only issues are missing EOF newlines in both changed files, introduced as a side-effect of the patch. Both Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[add_request called] --> B{assert endpoint contains /heartbeat}
B --> C{assert data is dict}
C --> D[_persistqueue.put QueuedRequest]
D -->|success| E[Heartbeat queued on disk]
D -->|OSError| F[logger.warning: Failed to queue request]
F --> G[Heartbeat dropped silently]
E --> H[Worker thread dispatches to server]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[add_request called] --> B{assert endpoint contains /heartbeat}
B --> C{assert data is dict}
C --> D[_persistqueue.put QueuedRequest]
D -->|success| E[Heartbeat queued on disk]
D -->|OSError| F[logger.warning: Failed to queue request]
F --> G[Heartbeat dropped silently]
E --> H[Worker thread dispatches to server]
Reviews (1): Last reviewed commit: "Catch OSError when persisting heartbeat ..." | Re-trigger Greptile |
| def register_bucket(self, bucket_id: str, event_type: str) -> None: | ||
| self._registered_buckets.append(Bucket(bucket_id, event_type)) | ||
| self._registered_buckets.append(Bucket(bucket_id, event_type)) No newline at end of file |
There was a problem hiding this comment.
Both changed files are missing a trailing newline at the end of file (
\ No newline at end of file in the diff). The register_bucket line in client.py lost its trailing newline as a side effect of this patch. POSIX requires text files to end with a newline, and many linters/diff tools will flag or silently alter files without one.
| def register_bucket(self, bucket_id: str, event_type: str) -> None: | |
| self._registered_buckets.append(Bucket(bucket_id, event_type)) | |
| self._registered_buckets.append(Bucket(bucket_id, event_type)) | |
| def register_bucket(self, bucket_id: str, event_type: str) -> None: | |
| self._registered_buckets.append(Bucket(bucket_id, event_type)) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| # Should not raise, the OSError should be caught internally and logged instead | ||
| rq.add_request("/api/0/buckets/test/heartbeat", {}) No newline at end of file |
There was a problem hiding this comment.
The new test function is also missing a trailing newline at end of file.
| # Should not raise, the OSError should be caught internally and logged instead | |
| rq.add_request("/api/0/buckets/test/heartbeat", {}) | |
| # Should not raise, the OSError should be caught internally and logged instead | |
| rq.add_request("/api/0/buckets/test/heartbeat", {}) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Problem
RequestQueue.add_request()writes heartbeats to a disk-backed queue viapersist-queue. If the disk runs out of space, that write raises an uncaughtOSError, crashing the program instead of failing gracefully.Fix
Wrapped the queue write in a
try/except OSError, logging a warning instead of crashing. A dropped heartbeat is an acceptable tradeoff compared to the whole process going down.Testing
test_add_request_disk_full, which simulates a disk-full scenario by makingpersistqueue.putraiseOSError, and confirmsadd_requesthandles it without crashing.pytest tests/test_requestqueue.py), all passing.ruff check .andmypy, no new issues introduced.Fixes #8