Skip to content

ESD-1676: Fix NAT gateway diagnostics poll treating in-progress 400 as fatal - #175

Open
Phil-Browne wants to merge 4 commits into
mainfrom
feature/ESD-1676-nat-diagnostics-poll-inprogress
Open

ESD-1676: Fix NAT gateway diagnostics poll treating in-progress 400 as fatal#175
Phil-Browne wants to merge 4 commits into
mainfrom
feature/ESD-1676-nat-diagnostics-poll-inprogress

Conversation

@Phil-Browne

@Phil-Browne Phil-Browne commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

While a NAT gateway route-diagnostics operation is still processing, the API returns an HTTP 400 with {"message":"The polling result for async mode is not ready yet"}; completion is a 200 with a data array that may legitimately be empty. The poll treated that in-progress 400 as fatal and bailed on the first iteration, and treated an empty 200 as "still processing" (the reverse of the real contract), so anything slower than the 2s initial delay failed.

  • Add an internal isNATGatewayDiagnosticsInProgress helper to detect the in-progress 400. It stays unexported on purpose: the signal is a message substring with no error code behind it, and other diagnostics endpoints report the same state differently, so it should not become public SDK surface.
  • pollDiagnosticsRoutes now keeps polling on that signal, returns any other error unchanged, and treats a 200 as the completed result including an empty slice.
  • Add poll-cadence override fields on the service (mirrors the MCR looking-glass poll) so the 400-then-200 transition is unit-tested deterministically without real sleeps.

Unit tests cover in-progress-then-complete, empty-complete, other-error-fatal, timeout, and caller-cancellation.

Contract verification

Verified against the megalith backend source (not just the SDK/spec):

  • In-progress: DiagnosticsRoutesClient throws RoutesNotAvailableException("The polling result for async mode is not ready yet") on NetAuto status PENDING/RUNNING; the v3 NAT gateway controller re-throws it, and the global handler maps any unannotated RuntimeException to HTTP 400 with that string in the message field. So the 400 + message match is the real contract.
  • Completion: COMPLETE status returns a 200, and an empty route list is a successful empty result, not "still processing".
  • The signal is in message, not errorCode/data (no errorCode is set on this path).
  • The equivalent v2 MCR endpoint reports the same state as an HTTP 202 instead, which is why this helper is scoped to NAT gateway rather than shared.

What I could not verify

  • The in-progress message is a hardcoded string constant in the backend with no stable errorCode behind it, and it is not in the OpenAPI spec (the endpoint documents only a 200 and a catch-all 4XX). A future wording change would silently break the substring match; keying on a stable errorCode would be more robust if the backend ever exposes one.

…s fatal

The async route-diagnostics operation endpoint reports "still processing"
with an HTTP 400 whose body is
{"message":"The polling result for async mode is not ready yet"} and
reports completion with a 200 carrying a data array that may legitimately
be empty. The poll treated the in-progress 400 as fatal and returned on
the first iteration, and treated an empty 200 as "still processing", the
reverse of the real contract.

Add IsDiagnosticsInProgressError (mirroring IsServiceNotFoundError) and
have pollDiagnosticsRoutes keep polling on that signal, return any other
error, and treat a 200 as the completed result including an empty slice.
Add poll-cadence override fields on the service (mirroring the MCR
looking-glass poll) so the transition is unit-tested deterministically.
Match the mcr_looking_glass sibling's per-field comment style.
…tion

Cover the pollDoneErr caller-context path (returns context.Canceled, not
the timeout sentinel) and assert the timeout test tolerated at least one
in-progress poll.
@Phil-Browne
Phil-Browne requested review from a team and mp1-tomclancy and removed request for a team July 21, 2026 22:58
The check matches a message substring on the non-standard 400 the NAT
gateway routes operation endpoint returns while a result is not ready.
There is no error code behind that signal, and other diagnostics
endpoints report the same state differently, so it should not be public
SDK surface. Renames IsDiagnosticsInProgressError to
isNATGatewayDiagnosticsInProgress.
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.

1 participant