Fix overlay SSE EPIPE handling#2781
Merged
Merged
Conversation
9e4ed8c to
380f554
Compare
…antics - Expose send() on SseStreamClient instead of the raw RxJS Subject - Restore clearAllStates() in resetAllOverlays so reset is a hard wipe - Drop no-op axios try/catch in plex-api uploadPoster - Use plain warn() + debug(error) per logging convention - RevertItemResult as discriminated string union
380f554 to
c7addd6
Compare
enoch85
approved these changes
Apr 27, 2026
enoch85
left a comment
Collaborator
There was a problem hiding this comment.
I approve this in current state.
Please test the latest pushes, and confirm it fixes your issue. Once approved, I'll merge this.
Thanks for the PR! 🚀
Collaborator
|
Decided to merge this anyway to avoid conflicts on another PR I'm working on touching the same files. It's also easier to confirm now that the changes are in |
maintainerr-automation Bot
added a commit
that referenced
this pull request
Apr 27, 2026
* fix(plex): bound request timeout and surface watch-history errors (#2773) The runtime PlexApi client had no socket timeout, so a wedged Plex request could hang the rule executor forever. Add PLEX_REQUEST_TIMEOUT_MS (30s) to bound it. Also align plexApi.getWatchHistory with the Jellyfin adapter's contract: errors propagate so callers can distinguish a real outage from a confirmed empty history (matches the design documented for the rules layer's NOT_EXISTS / missing-value handling). Removes silent .catch(() => null) swallowers in seenBy, sw_allEpisodesSeenBy and collection_siblings_lastViewedAt that previously turned transient lookup failures into "viewed by no one". Refs #2771 * fix(overlays): preset edit, cron discoverability, tab gating (#2775) * fix(plex): bound request timeout and surface watch-history errors The runtime PlexApi client had no socket timeout, so a wedged Plex request could hang the rule executor forever. Add PLEX_REQUEST_TIMEOUT_MS (30s) to bound it. Also align plexApi.getWatchHistory with the Jellyfin adapter's contract: errors propagate so callers can distinguish a real outage from a confirmed empty history (matches the design documented for the rules layer's NOT_EXISTS / missing-value handling). Removes silent .catch(() => null) swallowers in seenBy, sw_allEpisodesSeenBy and collection_siblings_lastViewedAt that previously turned transient lookup failures into "viewed by no one". Refs #2771 * fix(overlays): resolve preset edit, cron discoverability, and tab gating issues - Fix #2768: preset edit now opens the editor with a copy-on-save flow (info description, "Save as copy" button, name modal) and remounts on id change so switching to /new from a preset URL no longer leaks state. - Add post-save modal prompting for a cron schedule when overlays are enabled with no schedule set, with a direct link to Job Settings. - Gate template tabs and Run Now / Reset on persisted overlay-enabled state; redirect direct nav to /overlays/templates when disabled. - Add Docs button on Overlay Settings linking to docs.maintainerr.info/overlays. - Add useOverlaySettings query + useUpdateOverlaySettings mutation mirroring useSettings/usePatchSettings, so the wrapper re-evaluates gating after save via cache propagation. - Extract BrandLink shared component for inline branded body-text links; swap manual styling in Metadata, Jobs, AddModal, Calendar, StorageMetrics. - Replace crontab.org with crontab.guru and dedupe per-row cron links into a single link in the Job Settings page description. - Suppress success alert on copy-on-save navigation (the URL change is the confirmation; the alert flashed for one frame and disappeared). - Sync workspace MCP config (.mcp.json + .vscode/mcp.json) to use --output-dir .playwright-mcp; document screenshot path requirement. * fix(ui): unblock tsc + fix the type errors it was hiding (#2776) apps/ui/tsconfig.json had ignoreDeprecations: "6.0", which is invalid for TypeScript 5.x and caused tsc to bail before type-checking. CI exited 0 without scanning anything, so eight pre-existing type errors went unreported. - Remove the invalid ignoreDeprecations option (no deprecated options remain in the tsconfig that need silencing). - Drop unused imports in CollectionInfo (DocumentTextIcon, CollectionLogDto, isMetaActionedByRule). - Add the missing MediaItem fields (guid, addedAt, providerIds, mediaSources, library) to the CollectionMediaPage test fixture. - Update the UseQueryResult / QueryClient mock casts in two specs to use the project's existing `as unknown as ...` double-cast pattern (matches 12 other specs). * ci(fider): tighten triage script and surface pre-existing matches in re-eval Re-evaluate workflow can now catch "feature already exists, user didn't know" cases that the daily triage's PR-prefix gate misses (e.g. foundational date-rule support that predates the feat:/fix: convention). - Lift sleep + the entire model-call layer (throttle, per-run budget, retry-with-backoff, retry-after handling, header logging) from fider-triage.mjs into a createModelCaller factory in fider-shared.mjs. - Replace the wrong "Copilot Free 15 RPM / 150 RPD" comment with the observed runner-token budget (1000 RPM / 1M TPM, no exposed daily cap) from a real re-eval run; pace 1 call/sec with an 800-call sanity ceiling. - BudgetExhaustedError now propagates through triagePost catches so main() aborts cleanly with a deferred-posts log instead of burning iterations into the consecutive-failure abort. - Drop the feat:/fix: PR-title prefix gate on the pre-existing path when forceReeval=true; daily run still enforces it for precision. - Re-eval workflow now sets CHECK_PRE_EXISTING=true so the relaxed path actually runs. - MAX_POSTS_PER_RUN 25 -> 100; consolidate the three judge*WithModel and build*Comment functions; collapse filterCandidates + filterPreExistingCandidates into one parametric filterPRs. Net: triage script 844 -> 649 lines (-23%), shared +117 for the lifted infra; behaviour preserved end-to-end (comment markers byte-identical, tag slugs unchanged, quote-validation rejection paths preserved). * ci(fider): @-mention OP and nudge to split bundled requests in possibly-pre-existing comment Both pre-existing flags from the latest re-eval run were partial matches caused by multi-ask FRs: the bot quoted a PR that delivered one bullet of a wishlist and tagged the post pre-existing. The new footer addresses the OP directly via @-mention (already returned on every post by /api/v1/posts) and asks them to split bundled requests so each can be triaged independently. The long maintainer-facing "lower confidence than possibly-completed" preamble that the OP didn't need is gone. Comment-marker idempotency means existing pre-existing comments will be edited (not duplicated) to the new text on the next re-eval run. * Fix overlay SSE EPIPE handling (#2781) * Fix overlay SSE EPIPE handling * refactor(overlays,sse): tighten SSE writer API and overlay revert semantics - Expose send() on SseStreamClient instead of the raw RxJS Subject - Restore clearAllStates() in resetAllOverlays so reset is a hard wipe - Drop no-op axios try/catch in plex-api uploadPoster - Use plain warn() + debug(error) per logging convention - RevertItemResult as discriminated string union --------- Co-authored-by: enoch85 <mailto@danielhansson.nu> --------- Co-authored-by: enoch85 <mailto@danielhansson.nu> Co-authored-by: Kristian Matthews-Kennington <kristian@matthews-kennington.com>
Contributor
|
🎉 This PR is included in version 3.9.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Collaborator
|
Seems still to be an issue: #2807 |
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
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.
Description & Design
This change fixes a server crash during overlay
Run Nowruns when poster restore/revert work hits a broken stream or transient Plex upload failure.The fix has two parts:
EPIPEwrite failures clean up the client instead of reachinguncaughtException.This approach keeps the fix narrow and server-side. It avoids changing public APIs, contracts, database schema, or UI behaviour, and follows the existing retry-safe overlay design: only clear backup/state after a confirmed restore.
Trade-offs and edge cases:
Related issue
Fixes #2780
AI-Assisted Development
This PR was AI-assisted using Codex.
AI assistance was used to trace the issue, inspect the overlay revert and SSE streaming paths, draft the implementation, and add focused regression tests. I reviewed the relevant server code paths, kept the changes scoped to the existing architecture, validated that backup/state preservation still works, and ran the project quality gates.
This solution fits the project's standards because it uses existing NestJS/RxJS patterns, avoids new dependencies, keeps changes small and reviewable, preserves retry-safe overlay behaviour, and adds targeted tests for the reported failure mode.
Checklist
How to test
Run lint and type checks:
yarn lintyarn check-typesRun the focused regression tests:
yarn --cwd apps/server jest --runTestsByPath src/modules/overlays/overlay-processor.service.spec.ts src/modules/events/events.controller.spec.ts src/modules/logging/logs.controller.spec.tsBuild the workspace:
yarn buildManual verification, if available:
Additional context
yarn buildpasses. The UI build may still print the existing Vite large chunk warning; this change does not introduce a new warning.