feat(core): report the HTTP status in API error messages - #92
Merged
juemerson-at-purestorage merged 2 commits intoAug 4, 2026
Merged
Conversation
ConvertTo-PfbApiError now appends "(HTTP nnn)" to the message it returns whenever the failure reached the array and came back with a status. Callers that consume these failures programmatically -- automated tests, log analysis, retry logic -- need the status to tell a malformed request (400) from a permission problem (403) from an endpoint absent at the connected REST version (404) from something transient worth retrying (503). The prose message alone often cannot: a FlashBlade 400 and 403 can both surface as nothing more specific than "Bad Request". The status was previously unavailable to those callers on exactly the failures where it matters most. The body-parsing branch REPLACES the message rather than extending it, so any error that returned a readable body -- i.e. every deliberate API rejection -- discarded the transport text that carried the status. Taken from the transport layer, never from the body's own "code" field. Those are different numbers that only sometimes agree: a write rejected for targeting the wrong array in a fleet returns code 13 inside an HTTP 400. Reading "code" would report a plausible but wrong status some of the time, which is worse than reporting none -- a missing status reads as unknown, a wrong one gets believed. Absent when there is genuinely no status: DNS failure, timeout, rejected cert. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Invoke-PfbApiRequest has two throw sites that both format via ConvertTo-PfbApiError, reached by different statuses. A 400 takes the plain else branch. A 401/403 on a reconnectable session reconnects and retries first, and when the retry also fails throws from inside that block -- where the record being formatted is the outer catch's original error rather than the retry's. Live testing covered the first branch and could not cover the second: the lab credential is a full array admin, so no read it can issue is refused. Mocked here rather than left implicitly assumed, and labelled as mocked so it is not mistaken for live evidence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Merged
dmann000
added a commit
that referenced
this pull request
Aug 4, 2026
Bumps ModuleVersion 2.2.0 -> 2.3.0, adds the 2.3.0 CHANGELOG entry, and refreshes the manifest ReleaseNotes highlight. This is the release-gating PR and should merge LAST, after the three reviewed-ready PRs it documents: #91 (array-connection remote_names / #64), #92 (HTTP status in errors), and #93 (RemoteDefaultExports suppression / #55). Includes two disclosed breaking changes (see CHANGELOG "Changed"): the Update-PfbArrayConnection -Name/-RemoteName alias collision, and New-PfbFileSystemReplicaLink -RemoteDefaultExports becoming [Nullable[bool]]. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.
What this changes
ConvertTo-PfbApiErrornow includes the HTTP status in the message it returns, as(HTTP nnn):Why
Anything consuming these failures programmatically — automated tests, log analysis, retry logic —
needs the status to tell apart cases that call for completely different responses: a malformed
request (400), a credential without permission (403), an endpoint absent at the connected REST
version (404), something transient worth retrying (503). The prose message alone frequently cannot
distinguish them; a FlashBlade 400 and 403 can both arrive as nothing more specific than
Bad Request.The status was previously unavailable on exactly the failures where it matters most. The
body-parsing branch replaces the message rather than extending it:
So every error that returned a readable JSON body — i.e. every deliberate API rejection — discarded
the transport text that carried the status. Only connection-level failures kept it, incidentally,
because nothing overwrote them.
Where the status comes from, and where it deliberately doesn't
Read from the transport layer (
Exception.Response.StatusCode), never from the error body's owncodefield. Those are different numbers that only sometimes agree — a write rejected for targetingthe wrong array in a fleet returns application code
13inside an HTTP 400. Readingcodewouldreport a plausible but wrong status some of the time, which is worse than reporting none: a missing
status reads as unknown, a wrong one gets believed. There's a test pinning that specific case.
Absent when there is genuinely no status to report — DNS failure, connection timeout, rejected
certificate. Range-checked so a response carrying no usable status contributes nothing rather than
(HTTP 0).This reuses the access pattern already in the same
catchblock for the 401/403 reconnect gate,rather than introducing a second idiom. The
[int]cast is left unguarded for the same reason thatone is: lines 146-149 perform the identical cast unconditionally at the top of the block, before
control can reach
ConvertTo-PfbApiError, so an exception with an uncastableStatusCodefailsthere first.
Verification
Live, against a lab FlashBlade (Purity//FB REST 2.26) — a malformed filter, which the array
rejects with a JSON error body:
That is the body-parsing branch, so it exercises the case that previously lost the status.
One branch could not be reached live, and is mocked instead.
Invoke-PfbApiRequesthas twothrow sites that both format through this function. A 400 takes the plain
else. A 401/403 on areconnectable session reconnects and retries first, and when the retry also fails it throws from
inside that block — where the record being formatted is the outer
catch's original error, not theretry's. The lab credential is a full array admin, so no read it can issue is refused; I probed 19
read cmdlets across admin, api-tokens, legal-holds, rapid-data-locking, KMIP, keytabs, sessions,
roles and SAML/OIDC without provoking a 403. The second commit pins that branch with a mocked test,
labelled in the test body as mocked so it isn't later mistaken for live evidence.
Test suite:
b84b1cb: 1701 passed / 0 failed (PowerShell 7)0848541: 17/17 on both PowerShell 7 and Windows PowerShell 5.1ConvertTo-PfbApiError.Tests.ps1goes 3 → 9 tests. The three existing exact-match assertions passunchanged — they build an exception with no
.Response, so no status is added and the oldexpected strings still describe real behaviour.
New coverage: the status on both message branches, the body-
code-is-not-the-status case, absentwhen there was no HTTP response, absent rather than
(HTTP 0)when a response carries no usablestatus, and a round-trip asserting the
(HTTP nnn)form stays machine-readable across 400/403/404/500/503.
Knock-on effect worth flagging
Three call sites interpolate this message into their own strings, so their text now carries the
status too:
Public/Connection/Connect-PfbArray.ps1:190and:294Private/Invoke-PfbApiTokenLogin.ps1:39No test asserts on their exact wording, and the change is an improvement there — a failed connect
now says whether it was refused or unreachable. Noting it because it's user-visible beyond the one
function.
Not included
No version bump and no CHANGELOG entry, per the project convention that those are the maintainer's
own decision.
🤖 Generated with Claude Code