Fix unreliable Sonarr/Radarr rename flow and improve error handling #13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The bug was that after mark_as_reencoded returned the updated video, we were still passing the old video struct to finalize_and_sync, which meant the refresh_and_rename call was using stale video data.
- Add retry logic with exponential backoff for getting renameable files - Validate video dimensions in MediaInfoExtractor before extraction - Improve error handling throughout the pipeline - Simplify rename flow by always renaming all renameable files - Extract rename logic into separate, testable functions - Update dependencies to latest versions Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
The rename flow was unreliable because it used fixed Process.sleep(5000) calls instead of waiting for commands to actually complete. Refresh commands can take 10-15+ seconds depending on server load. Changes: - Add command polling functions to sonarr.ex and radarr.ex: - get_command_status/1 - checks command status by ID - wait_for_command/3 - polls until command completes - refresh_series_and_wait/2 and refresh_movie_and_wait/2 - Update Sync.refresh_operations to use refresh_*_and_wait - Update rename execution to wait for rename command to complete - Fix refresh_and_rename_all_series to wait for refresh completion - Add integration test scripts for debugging rename issues The flow now properly waits for each async command to complete before proceeding, making it reliable regardless of server response times. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The post-processor now spawns the refresh_and_rename in a background task via TaskSupervisor instead of blocking. This prevents the encoding pipeline from waiting 10-20+ seconds for Sonarr/Radarr commands. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug in the Sonarr/Radarr rename flow by replacing unreliable fixed-delay sleeps with proper command status polling. The changes ensure that refresh operations complete before attempting to rename files, preventing race conditions that caused the rename flow to fail.
Changes:
- Replaced
Process.sleep(5000)with actual command completion polling via newwait_for_commandandrefresh_*_and_waitfunctions - Moved refresh-and-rename operations to background tasks to avoid blocking the encoding pipeline
- Added validation for video dimensions (width/height) during MediaInfo extraction to handle corrupted data gracefully
- Added three integration test scripts for debugging Sonarr/Radarr rename flows
Reviewed changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/test_sonarr_rename.exs |
New standalone integration test for Sonarr rename functionality |
scripts/test_radarr_rename.exs |
New standalone integration test for Radarr rename functionality |
scripts/test_app_rename.exs |
New test script for application-level rename flow |
lib/reencodarr/services/sonarr.ex |
Added command polling functions, removed fixed delays, simplified rename logic |
lib/reencodarr/services/radarr.ex |
Added command polling functions, removed fixed delays, simplified rename logic |
lib/reencodarr/sync.ex |
Updated to use new wait functions, added error handling for invalid dimensions |
lib/reencodarr/post_processor.ex |
Changed to spawn refresh-and-rename as async background task |
lib/reencodarr/media/video.ex |
Added error handling for MediaInfo extraction failures |
lib/reencodarr/media/media_info_utils.ex |
Added dimension validation returning error tuples for invalid data |
lib/reencodarr/media/media_info_extractor.ex |
Added dimension validation with warning logs |
lib/reencodarr/analyzer/processing/pipeline.ex |
Updated to handle error tuples from extraction, improved logging |
lib/reencodarr/ab_av1/progress_parser.ex |
Fixed regex to support GiB/MiB file size formats |
mix.lock |
Routine dependency updates |
flake.lock |
Routine nixpkgs update |
- Use function clauses with guards instead of cond for rename_files validation - Simplify parse_renameable_files by extracting collect_results helper - Remove trivial determine_files_to_rename function - Simplify refresh_operations by removing unnecessary else blocks - Consolidate refresh_and_rename_from_video with service type dispatch - Extract coerce_to_integer helper to reduce duplication - Fix default parameter placement in rename_movie_files These changes improve readability and follow Elixir conventions more closely. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Changes in response to Copilot PR review: 1. **Task error handling** - Handle Task.Supervisor.start_child results properly to detect and log task startup failures instead of ignoring them. Extracted to spawn_refresh_and_rename_task helper to reduce nesting. 2. **Exponential backoff** - Implement actual exponential backoff (1s, 2s, 4s) for retry_get_renameable_files instead of fixed 3s delay. This reduces unnecessary waiting when files are ready quickly. 3. **Test coverage** - Add test files documenting the integration tests: - test/reencodarr/services/sonarr_test.exs - test/reencodarr/services/radarr_test.exs These reference scripts/test_app_rename.exs which provides comprehensive integration test coverage for wait_for_command, refresh_*_and_wait, and exponential backoff logic with live API responses. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ocumentation Fixes addressing 10 new Copilot review threads: 1. Width/height validation: Check for negative values (w <= 0 or h <= 0) instead of just zero - Updated both media_info_utils.ex and media_info_extractor.ex - Defensive programming to catch corrupted MediaInfo data 2. Remove unused file_ids parameter from public API - sonarr.ex: Removed from rename_files/1 (was ignored with underscore) - radarr.ex: Removed from rename_movie_files/1 (was ignored with underscore) - Updated all call sites to use parameter-less versions - Clarifies that function always renames all renameable files 3. Add comprehensive exponential backoff documentation - sonarr.ex: Added comments explaining 1s, 2s, 4s, 8s delay pattern - radarr.ex: Same documentation for consistency - Comments explain the mathematical formula: base_delay * 2^(attempt-1) 4. Simplified sync.ex refresh_operations - Removed empty list parameters from Sonarr/Radarr calls - Code now matches the simplified function signatures Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes a long-standing bug where file renames weren't being detected by Sonarr/Radarr after re-encoding. The root cause was using fixed
Process.sleep(5000)calls instead of actually waiting for async commands to complete.Key changes:
wait_for_command/3andrefresh_*_and_wait/2functions to wait for Sonarr/Radarr commands to completeWhy This Matters
Previously, the flow was:
Now it's:
The entire operation runs in a background task, so it doesn't block the encoding pipeline.
Testing
Added three standalone test scripts:
scripts/test_sonarr_rename.exs- Tests Sonarr rename flow with file manipulationscripts/test_radarr_rename.exs- Tests Radarr rename flow with file manipulationscripts/test_app_rename.exs- Tests actual application code usingSync.refresh_and_rename_from_videoAll tests pass with both services. The fix has been verified to reliably rename files regardless of server response time.
Files Changed
lib/reencodarr/services/sonarr.ex- Add command polling, update rename to waitlib/reencodarr/services/radarr.ex- Add command polling, update rename to waitlib/reencodarr/sync.ex- Userefresh_*_and_waitfunctions instead of fixed sleeplib/reencodarr/post_processor.ex- Run refresh_and_rename async in background task🤖 Generated with Claude Code