Classify 4xx (except 429) as terminal in RestSend retry policy

[allenrobel]

Related Issue(s)

Addresses the 4xx half of #457 (deliberately does NOT close it — the 5xx retry question remains open for discussion there).

Proposed Changes

Implements the independently-shippable piece proposed in #457's field-report comment: a 4xx response proves the request reached the application and was rejected, so replaying the identical request cannot succeed. Previously a deterministic 400 (e.g. an immediately-rejected fabric mutation) was resubmitted every 5 seconds for the full 300-second RestSend budget, delaying module failure by 5 minutes with no user-facing knob to shorten it.

• ResponseHandler._is_terminal_client_error(): 400–499 minus 429. Rate limiting (429) is the one transient 4xx and stays retryable. No retryable 4xx is documented for any ND 4.2.1 endpoint; the dcnm-era retry-on-400 cases do not carry over.
• Mutations (POST/PUT/DELETE): the retryable classification introduced by Detect Multi-status per-item failures in NdV1Strategy (#295) #398 now also excludes terminal client errors. 5xx keeps the historical retry behavior — that half of Discuss: should RestSend retry mutations (POST/PUT/DELETE) that fail with non-success codes? #457 is untouched.
• GET: results now carry retryable, applying the same 4xx rule per the issue's scope note (a deterministic 400 on a GET burned the same budget). 5xx GETs remain retryable for eventual-consistency polling, and the 404 not-found-is-success contract is unchanged.
• RestSend needs no changes — the terminal break from Detect Multi-status per-item failures in NdV1Strategy (#295) #398 already consumes retryable verb-agnostically.

Test changes: five new ResponseHandler tests (01500–01540: POST 400 terminal, POST 429 retryable, GET 400 terminal, GET 404 contract, DELETE 404 terminal) and a deliberate rewrite of 01440, which previously pinned the "GET results carry no retryable key" behavior this PR removes.

Test Notes

• Full unit suite passes in the nd-dev container machine: 3983 passed (ndpytest tests/unit/)
• pylint 10.00/10 on the changed module; black/isort clean; mypy reports only the two errors pre-existing on develop

Cisco Nexus Dashboard Version

4.2.1

Related ND API Resource Category

• analyze
• infra
• manage
• onemanage
• other

Checklist

• Latest commit is rebased from develop with merge conflicts resolved
• New or updates to documentation has been made accordingly
• Assigned the proper reviewers

🤖 Generated with Claude Code

https://claude.ai/code/session_019XZiWNrVHYXbeNXRM8kUrz

[mikewiebe]

Medium: Preserve retryable 4xx responses instead of terminalizing the entire class

Issue

_is_terminal_client_error() treats every 4xx except 429 as a definitive application rejection for every verb. That is too broad: HTTP 408, 421, and 425 explicitly support retry, and the bundled ND OpenAPI documents 409 on safe GET operations whose conflict is tied to the resource's current state.

Evidence

• plugins/module_utils/rest/response_handler_nd.py line 190
  classifies the full 400..499 range as terminal with only 429 excluded.
• plugins/module_utils/rest/response_handler_nd.py lines 225-228
  applies that same predicate to GET, even though GET is safe/idempotent and its existing retries also serve polling and eventual-consistency workflows.
• plugins/module_utils/rest/rest_send.py lines 377-384
  immediately exits the retry loop when the handler returns retryable=False.
• The bundled Manage OpenAPI v1.1.411 spec documents 409 on 23 operations, including five GETs: GET /anomalyRules/postProcessingRules, GET /links, GET /links/{linkId}, GET /logicalLinks, and GET /remoteFabrics. Its shared response says the conflict is with the resource's current state; the spec contains no retryability metadata that supports blanket terminalization.
• RFC 9110 sections 15.5.9 and 15.5.20 permit retrying 408 and 421 respectively; 421 can be retried even for a non-idempotent method when a different connection is used. RFC 8470 section 5.2 expects an automatic retry after 425 outside early data.
• A local check against the exact PR head returned retryable=False for GET 408, 409, and 425, while only GET 429 remained retryable.

Existing PR overlap

No matching existing PR comment found.

Existing open issue overlap

Related open issue: #457 is the design source for this PR and proposes the same blanket 4xx-except-429 rule. It does not track the protocol-level 408/421/425 exceptions or the OpenAPI-documented GET 409 counterexample, so this finding narrows and corrects that policy rather than duplicating tracked work.

Impact

A concrete ND scenario is topology reconciliation after a switch is added or updated. An Ansible module requests GET /links while ND is still reconciling the fabric and receives the OpenAPI-documented 409 Conflict because the link data is temporarily unavailable in its current state. A retry a few seconds later can return 200 OK with the completed link data.

develop: GET /links -> 409 Conflict -> retry -> 200 OK -> task succeeds
PR #502: GET /links -> 409 Conflict -> stop retrying -> task fails

The operator therefore sees a failed automation job even though nothing is permanently wrong, and running the same playbook again moments later can succeed. This is a representative scenario inferred from ND's OpenAPI contract, not a live-testbed reproduction. The same blanket classification also suppresses standards-defined retry handling for 408, 421, and 425, and because ResponseHandler is shared, the regression affects every module using RestSend.

Suggested fix

Split safe GET policy from mutation policy and replace the blanket range check with explicit, evidence-backed classifications. At minimum, keep 408 and 425 retryable; preserve retry/polling for safe GET 409; and handle 421 by reopening or changing the connection before retrying. Put the version-specific decision on the injected ResponseValidationStrategy (or another policy object), matching this file's documented extension point, and add handler plus RestSend tests for the retained retry cases.

---