Skip to content

Fix WRONGTYPE error and add memory leak detection tests#153

Merged
chrisguidry merged 1 commit intomainfrom
fix-wrongtype-error-and-memory-leak-tests
Aug 18, 2025
Merged

Fix WRONGTYPE error and add memory leak detection tests#153
chrisguidry merged 1 commit intomainfrom
fix-wrongtype-error-and-memory-leak-tests

Conversation

@chrisguidry
Copy link
Owner

Summary

  • Fixes a Redis WRONGTYPE error that occurred when the worker scheduler tried to HSET on known task keys that existed as strings from legacy implementations
  • Adds systematic Redis key counting tests that can detect memory leaks by comparing key counts before and after task operations
  • Improves code organization by hoisting imports to top of test file

What was the problem?

The error occurred because we were redundantly storing stream message IDs in known task keys - this wasn't needed for the cancellation functionality to work. When legacy docket versions created known task keys as simple string values, the new scheduler would fail trying to HSET on those string keys.

What changed?

  • Remove redundant HSET of stream_message_id in worker scheduler Lua script
  • Add memory leak detection tests with KeyCountChecker helper class that covers successful tasks, failed tasks, and cancelled tasks
  • Add regression test for the original WRONGTYPE error scenario
  • Hoist all imports to top of test file per code style guidelines

Test plan

The new tests systematically verify that Redis keys are properly cleaned up in all scenarios and can detect memory leaks by counting keys before and after operations. We validated the tests work by temporarily introducing bugs and confirming they catch the issues.

🤖 Generated with Claude Code

Fixes a Redis WRONGTYPE error that occurred when the worker scheduler
tried to HSET on known task keys that existed as strings from legacy
implementations. The error happened because we were redundantly storing
stream message IDs in known task keys - this wasn't needed for the
cancellation functionality to work.

Also adds systematic Redis key counting tests that can detect memory
leaks by comparing key counts before and after task operations. These
tests cover successful tasks, failed tasks, and cancelled tasks to
ensure proper cleanup in all scenarios.

Changes:
- Remove redundant HSET of stream_message_id in worker scheduler script
- Add comprehensive memory leak detection tests with KeyCountChecker helper
- Hoist all imports to top of test file per code style guidelines
- Add regression test for the original WRONGTYPE error scenario

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions
Copy link

📚 Documentation has been built for this PR!

You can download the documentation directly here:
https://github.com/chrisguidry/docket/actions/runs/17039578519/artifacts/3787928636

@codecov-commenter
Copy link

codecov-commenter commented Aug 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (9dc405c) to head (07f8b1a).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #153   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           31        31           
  Lines         4411      4495   +84     
  Branches       244       246    +2     
=========================================
+ Hits          4411      4495   +84     
Flag Coverage Δ
python-3.12 100.00% <100.00%> (ø)
python-3.13 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/docket/worker.py 100.00% <ø> (ø)
tests/test_worker.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chrisguidry chrisguidry merged commit c2b8187 into main Aug 18, 2025
16 checks passed
@chrisguidry chrisguidry deleted the fix-wrongtype-error-and-memory-leak-tests branch August 18, 2025 11:45
chrisguidry added a commit that referenced this pull request Aug 18, 2025
…replacement (#154)"

This reverts commit 85f293c.

Revert "Fix WRONGTYPE error and add memory leak detection tests (#153)"

This reverts commit c2b8187.

Revert "Fix race condition in task replacement where duplicate executions occur (#151)"

This reverts commit 9dc405c.
@chrisguidry chrisguidry mentioned this pull request Aug 18, 2025
chrisguidry added a commit that referenced this pull request Aug 18, 2025
These fixes seem to get at least one of my systems into a state where
it's infinitely looping on perpetual tasks. This may have been due to
some of the intermediate problems in 0.9.0 and 0.9.1, but I don't want
to take that chance. We'll come back and revisit this.

This reverts commit 85f293c.

Revert "Fix WRONGTYPE error and add memory leak detection tests (#153)"

This reverts commit c2b8187.

Revert "Fix race condition in task replacement where duplicate
executions occur (#151)"

This reverts commit 9dc405c.
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