fix(cluster): surface named-task lookup failures as errors instead of silent partial results#42
fix(cluster): surface named-task lookup failures as errors instead of silent partial results#42tablackburn wants to merge 3 commits into
Conversation
… silent partial results Previously, Get-StmClusteredScheduledTaskInfo silently emitted partial results (TaskName-only) when a named task could not be resolved on a cluster, and Get-StmClusteredScheduledTask fired a misleading "configure your cluster" warning for what was actually a single-named-task miss. Three changes across two files: - Get-StmClusteredScheduledTask.ps1: when -TaskName is given and the cluster returns zero, write a structured ClusteredScheduledTaskNotFound error instead of a cluster-wide warning. Bulk path (no -TaskName) keeps the warning. - Get-StmClusteredScheduledTask.ps1: detect stage-B misses (cluster registry has the task but the owner node didn't return it) and write a structured ClusteredScheduledTaskOwnerLookupFailed error per missing task. Suppresses the raw cmdletization error leak from the per-name Get-ScheduledTask call. - Get-StmClusteredScheduledTaskInfo.ps1: when -TaskName is given and the inner lookup is empty, write a structured ClusteredScheduledTaskNotResolvable error. Bulk path keeps the warning. Five new Pester tests across the two .Tests.ps1 files cover both error paths and verify the preserved bulk-path warnings. Validated red-then-green: new tests fail against pristine module, pass with patches. Also validated end-to-end against a real failover cluster in the integration lab. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR makes named-task misses produce structured ObjectNotFound errors (with ErrorId/RecommendedAction) and adds explicit owner-lookup mismatch errors; bulk queries keep warning behavior. Tests and integration calls updated to cover and tolerate the new behaviors. ChangesClustered task query error handling
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR replaces silent/misleading behavior in Get-StmClusteredScheduledTask and Get-StmClusteredScheduledTaskInfo with structured non-terminating errors when a specifically named clustered task cannot be resolved, while preserving the existing warning behavior on bulk (no -TaskName) calls. It also suppresses the raw cmdletization error leak from Get-ScheduledTask and re-emits a structured ClusteredScheduledTaskOwnerLookupFailed error per missing task.
Changes:
- Emit
ClusteredScheduledTaskNotFound(stage-A) andClusteredScheduledTaskOwnerLookupFailed(stage-B) errors fromGet-StmClusteredScheduledTaskfor named-task misses. - Emit
ClusteredScheduledTaskNotResolvablefromGet-StmClusteredScheduledTaskInfowhen a named lookup returns nothing. - Add 5 Pester tests covering named/bulk paths and the stage-B miss.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| ScheduledTasksManager/Public/Get-StmClusteredScheduledTask.ps1 | Adds structured errors for stage-A (task name not in cluster registry) and stage-B (owner node didn't return registered task) misses; bulk path warning preserved. |
| ScheduledTasksManager/Public/Get-StmClusteredScheduledTaskInfo.ps1 | Adds structured ClusteredScheduledTaskNotResolvable error when -TaskName is supplied but inner lookup is empty; bulk path warning preserved. |
| tests/Get-StmClusteredScheduledTask.Tests.ps1 | New tests for stage-A error, bulk warning preservation, and stage-B owner-lookup error. |
| tests/Get-StmClusteredScheduledTaskInfo.Tests.ps1 | New tests for the named-task error and bulk warning preservation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… contract Drop the stale "Should warn" unit test — its scenario is now covered by the PR's new structured-error tests (`ClusteredScheduledTaskOwnerLookupFailed`), and its old warning-based assertion no longer matches the post-#42 behavior. Switch four integration-test cleanup probes from `-EA SilentlyContinue` to `-EA Ignore`. `Ignore` does not pollute `$Error`; `SilentlyContinue` does. The polluted `$Error` was leaking across the PSRemoting boundary into psake's `$ErrorActionPreference = 'Stop'` scope and failing the build at `build.psake.ps1:266` after all 19 integration tests had passed. Probe-then-act pattern is preserved (Unregister-* is still terminating on missing tasks); collapsing to one-line idempotent cleanup is tracked in #44. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…ify step Disable-StmClusteredScheduledTask's verify-after-unregister called Get-StmClusteredScheduledTask with -ErrorAction Stop, expecting a silent null when the task was successfully unregistered. After this PR introduced the new ClusteredScheduledTaskNotFound error on the named-task path, that Get call now throws terminating on the SUCCESS case (task absent), which the surrounding catch block wraps as UnregisterFailed and re-throws. In the integration test, the "Should disable" Pester test still reported as passed (AutomatedLab's Invoke-LabCommand appears to swallow terminating errors from the remote lab session), but the error accumulated in the session's error stream and surfaced ~2s after Pester finished, at the outer Invoke-Command boundary in build.psake.ps1:266 where psake's $ErrorActionPreference = 'Stop' promoted it to a build failure. Empirically verified locally: Get-StmClusteredScheduledTask with the exact splat Disable used (-EA Stop, -WA SilentlyContinue) throws terminating with FQEI ClusteredScheduledTaskNotFound when the task is missing. Switch to -ErrorAction Ignore on the verify-Get: a missing task here is the SUCCESS condition. The existing `$taskExists = $null -ne $task` check correctly drives the success/failure branch without depending on the producer's error contract. Audit of other internal Get-StmClusteredScheduledTask callers: - Import-StmClusteredScheduledTask: -EA Stop, but wraps in try/catch that treats throw as 'doesn't exist'. Robust to both old and new behavior. - Enable/Export/Set/Start/Stop/Wait/Get-Info: all expect the task to exist; surfacing 'not found' as an error is the correct behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Get-StmClusteredScheduledTaskInfowas silently emitting partial results (TaskName-only) when a named task could not be resolved on a cluster.Get-StmClusteredScheduledTaskwas firing a misleading "configure your cluster" warning for what was actually a single named-task miss.CmdletizationQuery_NotFound_TaskNameerrors to the console.Changes
Get-StmClusteredScheduledTask.ps1-TaskNameis given andGet-ClusteredScheduledTaskreturns zero, write a structuredClusteredScheduledTaskNotFoundnon-terminating error (ObjectNotFoundcategory,TargetObject = $TaskName). Bulk path (no-TaskName) keeps the existing warning unchanged.Get-ScheduledTask, write a structuredClusteredScheduledTaskOwnerLookupFailederror. Suppresses the raw cmdletization error leak.Get-StmClusteredScheduledTaskInfo.ps1-TaskNameis given and the inner lookup is empty, write a structuredClusteredScheduledTaskNotResolvablenon-terminating error. Bulk path keeps the existing warning unchanged.Behavior before vs after
Calling the user's loop pattern (
$names | ForEach-Object { Get-StmClusteredScheduledTaskInfo -Cluster X -TaskName $_ }) with a mix of valid and invalid task names:ClusteredScheduledTaskNotFound,Get-StmClusteredScheduledTaskandClusteredScheduledTaskNotResolvable,Get-StmClusteredScheduledTaskInfoCmdletizationQuery_NotFound_TaskNameleakClusteredScheduledTaskOwnerLookupFailederror, no red leakTest plan
./build.ps1 UnitTest— all 61 tests pass (5 new tests in this PR)STMCLUSTERon the integration lab withAnyNode,ClusterWide, and an engineered stage-B missTests added
Get-StmClusteredScheduledTask.When a named task is not found in the cluster (Issue 1 / stage-A miss).Should write a structured non-terminating error (not a warning) when -TaskName is givenGet-StmClusteredScheduledTask.When a named task is not found in the cluster (Issue 1 / stage-A miss).Should still emit a warning (and no error) when -TaskName is omitted (bulk path)Get-StmClusteredScheduledTask.When the cluster claims an owner but the owner does not return the task (Issue 1 / stage-B miss).Should write a structured ClusteredScheduledTaskOwnerLookupFailed errorGet-StmClusteredScheduledTaskInfo.When a named task cannot be resolved (Issues 1 + 2).Should write a structured non-terminating error (not a warning) when -TaskName is givenGet-StmClusteredScheduledTaskInfo.When a named task cannot be resolved (Issues 1 + 2).Should still emit a warning (and no error) when -TaskName is omitted (bulk path)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests