Skip to content

feat: Replace 401 errors with actionable token recovery guidance#39

Merged
tablackburn merged 3 commits into
mainfrom
feature/improve-401-error-guidance
Apr 27, 2026
Merged

feat: Replace 401 errors with actionable token recovery guidance#39
tablackburn merged 3 commits into
mainfrom
feature/improve-401-error-guidance

Conversation

@tablackburn
Copy link
Copy Markdown
Owner

@tablackburn tablackburn commented Apr 27, 2026

Summary

  • Plex API 401 responses now throw a message naming the cmdlets a user can run to recover (Update-PatServerToken, Get-PatStoredServer, explicit -Token) instead of just surfacing the raw HTTP status.
  • Detection lives centrally in Invoke-PatApi, so every public cmdlet that hits the Plex API picks up the guidance automatically.
  • Fixes a latent unit test that constructed [System.Net.WebException] with an invalid 4-arg overload — the constructor failure was masking the actual assertion, so the test was a false positive.

Before

Failed to resolve section name: Failed to get Plex library information: Error invoking Plex API:
Response status code does not indicate success: 401 (Unauthorized).

After

Failed to resolve section name: Failed to get Plex library information: Plex API returned 401
Unauthorized. The authentication token is missing, expired, or invalid. To resolve: refresh the
token with 'Update-PatServerToken' (use -Name to target a non-default server), list configured
servers with 'Get-PatStoredServer', or pass an explicit -Token parameter to the cmdlet you are
calling.

The outer Failed to resolve section name: / Failed to get Plex library information: wrappers from upstream catch blocks are intentionally left alone — that's a separate readability issue worth addressing on its own.

Test plan

  • pwsh -File ./build.ps1 -Task Test — full suite green (psake exit 0)
  • New Invoke-PatApi unit tests cover both the WebException path and the raw-message path
  • Existing 401 retry/non-retry tests still pass (message change preserves *401* substring)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved Plex API error handling: 401/Unauthorized now show clear, actionable token-recovery guidance and preserve the original error text; other errors retain a consistent "Error invoking Plex API" prefix.
  • Documentation

    • Added guidance for refreshing/supplying server tokens (instructions referencing Update-PatServerToken and Get-PatStoredServer) and CI pre-flight token validation.
  • Tests

    • Expanded unit tests for 401/Unauthorized parsing and the new user-facing error messages.

Plex API 401 responses now throw a message naming the cmdlets users can
run to recover (Update-PatServerToken, Get-PatStoredServer, -Token) instead
of bubbling up the raw HTTP status. Detection happens centrally in
Invoke-PatApi so every public cmdlet that hits the Plex API benefits.

Also fixes a latent test that constructed a [WebException] with an invalid
4-arg overload — the constructor failure was masking the assertion.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 27, 2026 15:56
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f4c0ddc3-c923-4797-bf38-c576963bd195

📥 Commits

Reviewing files that changed from the base of the PR and between 078d79f and 134532d.

📒 Files selected for processing (1)
  • CHANGELOG.md
✅ Files skipped from review due to trivial changes (1)
  • CHANGELOG.md

📝 Walkthrough

Walkthrough

Detects 401/Unauthorized responses in Invoke-PatApi and throws an authentication guidance message naming recovery commands; unit tests expanded to validate 401 detection, preservation of original error text, and that non-401 Unauthorized exceptions use the generic error path.

Changes

Cohort / File(s) Summary
Authentication Error Handling
PlexAutomationToolkit/Private/Invoke-PatApi.ps1
On non-transient or final retry failures capture the exception message, regex-match \b401\b/Unauthorized, and throw a targeted auth guidance message referencing Update-PatServerToken, Get-PatStoredServer, or passing -Token. Other errors throw Error invoking Plex API: <message>.
Unit Tests
tests/Unit/Private/Invoke-PatApi.tests.ps1
Reworked tests to mock 401 via multiple thrown formats, assert token-recovery guidance appears and original error text is preserved, verify non-401 Unauthorized exceptions use the generic error path; some tests set -MaxRetries 1 -BaseDelaySeconds 0 to constrain retry timing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Poem

🐇 I hopped through errors, sniffed a 401 trace,
Found a missing token in a shadowy place.
"Update-PatServerToken," I gently plea,
"Or Get-PatStoredServer — or pass -Token to me!"
A carrot, a hint, and a hop to set pace 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: improving 401 error handling with actionable token recovery guidance. This is the primary focus across all modified files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/improve-401-error-guidance

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the internal Plex API invocation wrapper to replace generic 401 Unauthorized failures with actionable token-recovery guidance, and adjusts unit tests to validate the new messaging and fix an invalid WebException construction.

Changes:

  • Add centralized 401 detection in Invoke-PatApi and throw a guidance-focused error message.
  • Update Invoke-PatApi unit tests to assert the new guidance and correct the prior invalid WebException constructor usage.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
PlexAutomationToolkit/Private/Invoke-PatApi.ps1 Adds special-case handling for 401-ish errors to provide token recovery instructions.
tests/Unit/Private/Invoke-PatApi.tests.ps1 Updates/extends tests to validate the new 401 guidance messaging paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread PlexAutomationToolkit/Private/Invoke-PatApi.ps1
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
PlexAutomationToolkit/Private/Invoke-PatApi.ps1 (1)

137-144: Consider tightening the 401 detection and preserving original error context.

Two small concerns with the new branch:

  1. Matching the bare word Unauthorized (independent of 401) can trigger the 401 guidance for unrelated errors whose message happens to contain that word (e.g., a 403 body or a custom Plex error string mentioning "Unauthorized access"). Pairing the two checks (require both 401-ish status and Unauthorized wording, or drop the lone Unauthorized clause) would reduce false positives without harming the intended cases — both test fixtures satisfy \b401\b already.
  2. The 401 path discards $errorMessage entirely, so the original wrapped exception text (URL, inner-exception details, etc.) is lost from the user-visible error. Appending the captured detail keeps the actionable guidance front-and-center while retaining diagnostics for support/debug scenarios.
♻️ Proposed adjustment
-                # 401 indicates a missing, expired, or invalid token. Surface
-                # actionable guidance instead of the raw HTTP error so callers
-                # know which cmdlets to run to recover.
-                if ($errorMessage -match '\b401\b' -or $errorMessage -match 'Unauthorized') {
-                    throw ("Plex API returned 401 Unauthorized. The authentication token is missing, expired, or invalid. " +
-                        "To resolve: refresh the token with 'Update-PatServerToken' (use -Name to target a non-default server), " +
-                        "list configured servers with 'Get-PatStoredServer', " +
-                        "or pass an explicit -Token parameter to the cmdlet you are calling.")
-                }
+                # 401 indicates a missing, expired, or invalid token. Surface
+                # actionable guidance instead of the raw HTTP error so callers
+                # know which cmdlets to run to recover.
+                if ($errorMessage -match '\b401\b') {
+                    throw ("Plex API returned 401 Unauthorized. The authentication token is missing, expired, or invalid. " +
+                        "To resolve: refresh the token with 'Update-PatServerToken' (use -Name to target a non-default server), " +
+                        "list configured servers with 'Get-PatStoredServer', " +
+                        "or pass an explicit -Token parameter to the cmdlet you are calling. " +
+                        "Original error: $errorMessage")
+                }

If you keep the Unauthorized clause for safety, consider also asserting on Update-PatServerToken in the second 401 test (currently it only asserts on the "missing, expired, or invalid" wording) so the cmdlet-naming contract is covered for both message shapes.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@PlexAutomationToolkit/Private/Invoke-PatApi.ps1` around lines 137 - 144, The
401 branch in Invoke-PatApi is too broad and discards diagnostic text; update
the conditional to only trigger when the status indicates 401 (e.g.
$errorMessage -match '\b401\b') or when both a 401-like status and the word
"Unauthorized" are present (avoid a lone 'Unauthorized' match), and modify the
thrown message to append the original $errorMessage (preserve
URL/inner-exception details) while keeping the user guidance (mentioning
Update-PatServerToken, Get-PatStoredServer or explicit -Token); also
adjust/update tests to assert the presence of the Update-PatServerToken guidance
in both 401 message shapes if you retain the 'Unauthorized' clause.
tests/Unit/Private/Invoke-PatApi.tests.ps1 (1)

116-131: Good fix on the WebException constructor and solid two-shape coverage.

Switching to the 1-arg [System.Net.WebException]::new(string) overload removes the silent constructor failure that was previously masking the assertion, and covering both the WebException-formatted message and the raw HttpRequestException-style "Response status code does not indicate success: 401 (Unauthorized)." string is exactly what's needed to lock down the regex in Invoke-PatApi.ps1 line 137. Nice.

One small suggestion: the second test (lines 124-131) only asserts on *token is missing, expired, or invalid*. Consider also asserting on *Update-PatServerToken* there (as the first test does) so the cmdlet-naming contract is enforced for both message shapes — that way a future change to the regex that accidentally only matches one shape can't silently drop the cmdlet guidance for the other.

♻️ Optional tighter assertion
             { Invoke-PatApi -Uri 'http://localhost:32400/test' -MaxRetries 1 -BaseDelaySeconds 0 } |
-                Should -Throw "*token is missing, expired, or invalid*"
+                Should -Throw "*token is missing, expired, or invalid*Update-PatServerToken*Get-PatStoredServer*"
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/Unit/Private/Invoke-PatApi.tests.ps1` around lines 116 - 131, Add an
extra assertion to the second test ("Should include token recovery guidance when
401 is returned") so it also verifies the cmdlet guidance appears: when mocking
the 401 string and invoking Invoke-PatApi, assert the thrown message contains
both the token guidance "*token is missing, expired, or invalid*" and the cmdlet
name "*Update-PatServerToken*" (same as the first test) to ensure the regex in
Invoke-PatApi.ps1 still surfaces the cmdlet guidance for the alternate error
message shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@PlexAutomationToolkit/Private/Invoke-PatApi.ps1`:
- Around line 137-144: The 401 branch in Invoke-PatApi is too broad and discards
diagnostic text; update the conditional to only trigger when the status
indicates 401 (e.g. $errorMessage -match '\b401\b') or when both a 401-like
status and the word "Unauthorized" are present (avoid a lone 'Unauthorized'
match), and modify the thrown message to append the original $errorMessage
(preserve URL/inner-exception details) while keeping the user guidance
(mentioning Update-PatServerToken, Get-PatStoredServer or explicit -Token); also
adjust/update tests to assert the presence of the Update-PatServerToken guidance
in both 401 message shapes if you retain the 'Unauthorized' clause.

In `@tests/Unit/Private/Invoke-PatApi.tests.ps1`:
- Around line 116-131: Add an extra assertion to the second test ("Should
include token recovery guidance when 401 is returned") so it also verifies the
cmdlet guidance appears: when mocking the 401 string and invoking Invoke-PatApi,
assert the thrown message contains both the token guidance "*token is missing,
expired, or invalid*" and the cmdlet name "*Update-PatServerToken*" (same as the
first test) to ensure the regex in Invoke-PatApi.ps1 still surfaces the cmdlet
guidance for the alternate error message shape.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 12c64db5-a5b2-4cea-bdd7-95cbdcf38e8e

📥 Commits

Reviewing files that changed from the base of the PR and between a2a1b1d and 91db037.

📒 Files selected for processing (2)
  • PlexAutomationToolkit/Private/Invoke-PatApi.ps1
  • tests/Unit/Private/Invoke-PatApi.tests.ps1

Address review feedback on PR #39:

- Drop the lone 'Unauthorized' regex clause; rely on \b401\b alone.
  Bare 'Unauthorized' could misfire on UnauthorizedAccessException, 403
  bodies that include the word, proxy/auth middleware errors, etc., and
  then incorrectly recommend Plex token recovery.
- Append 'Original error: <message>' to the 401 guidance so URL/inner-
  exception detail is retained for support and debugging.
- Tighten the raw-string 401 test to also assert on Update-PatServerToken
  so the cmdlet-naming contract is enforced for both message shapes.
- Add a regression test asserting that an UnauthorizedAccessException-
  style message does NOT trigger the 401 guidance.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/Unit/Private/Invoke-PatApi.tests.ps1 (1)

116-131: Lock in the -Token recovery-path contract in the 401 assertions.

These tests validate cmdlet guidance, but they currently don’t assert the explicit -Token fallback mentioned by the runtime message contract.

Suggested assertion hardening
-            { Invoke-PatApi -Uri 'http://localhost:32400/test' } | Should -Throw "*401 Unauthorized*Update-PatServerToken*Get-PatStoredServer*"
+            { Invoke-PatApi -Uri 'http://localhost:32400/test' } | Should -Throw "*401 Unauthorized*Update-PatServerToken*Get-PatStoredServer*-Token parameter*"
...
-                Should -Throw "*token is missing, expired, or invalid*Update-PatServerToken*Get-PatStoredServer*"
+                Should -Throw "*token is missing, expired, or invalid*Update-PatServerToken*Get-PatStoredServer*-Token parameter*"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/Unit/Private/Invoke-PatApi.tests.ps1` around lines 116 - 131, The
401-related tests must assert the explicit "-Token" recovery-path hint in the
thrown guidance: update both Should -Throw expectations in the Invoke-PatApi
tests to include the "-Token" fragment (e.g., ensure the first test's pattern
contains "-Token" along with "401 Unauthorized", "Update-PatServerToken" and
"Get-PatStoredServer", and likewise add "-Token" to the second test's pattern
that already checks for "token is missing, expired, or invalid",
"Update-PatServerToken" and "Get-PatStoredServer") so the tests lock in the
-Token fallback contract for Invoke-PatApi.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/Unit/Private/Invoke-PatApi.tests.ps1`:
- Around line 116-131: The 401-related tests must assert the explicit "-Token"
recovery-path hint in the thrown guidance: update both Should -Throw
expectations in the Invoke-PatApi tests to include the "-Token" fragment (e.g.,
ensure the first test's pattern contains "-Token" along with "401 Unauthorized",
"Update-PatServerToken" and "Get-PatStoredServer", and likewise add "-Token" to
the second test's pattern that already checks for "token is missing, expired, or
invalid", "Update-PatServerToken" and "Get-PatStoredServer") so the tests lock
in the -Token fallback contract for Invoke-PatApi.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d9b8584b-ce05-44f1-9785-6bfe8e348312

📥 Commits

Reviewing files that changed from the base of the PR and between 91db037 and 078d79f.

📒 Files selected for processing (2)
  • PlexAutomationToolkit/Private/Invoke-PatApi.ps1
  • tests/Unit/Private/Invoke-PatApi.tests.ps1
🚧 Files skipped from review as they are similar to previous changes (1)
  • PlexAutomationToolkit/Private/Invoke-PatApi.ps1

The [Unreleased] section had drifted empty even though user-facing work
landed since 0.10.3. Backfill the entries so the next release captures
them and so users browsing the changelog can see what's coming.

- Added: Update-PatServerToken (#31) — interactive/direct token refresh
  with vault or inline storage and post-update verification
- Added: CI integration token validation pre-flight step (#31) — fast
  actionable failure when PLEX_TOKEN is expired
- Changed: actionable 401 token recovery guidance from this PR (#39)

Pure CI/dev-tooling churn (#34/#35/#36/#37/#38) intentionally omitted —
Keep a Changelog focuses on user-facing changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tablackburn tablackburn merged commit 9b04a27 into main Apr 27, 2026
15 checks passed
@tablackburn tablackburn deleted the feature/improve-401-error-guidance branch April 27, 2026 21:38
@tablackburn tablackburn mentioned this pull request Apr 27, 2026
4 tasks
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