Skip to content

execute() silently discards HTTP 4xx/5xx responses (Response truthiness), making transport faults indistinguishable from network failures #174

Description

@nadermx

Summary

APIOperationBase.execute() discards any HTTP response with a status >= 400,
and reports it to the caller identically to a connection failure. The status
code and body are never surfaced, and by default nothing is logged at a level
anyone sees.

Where

authorizenet/apicontrollersbase.py (current master), in execute():

try:
    self._httpResponse = requests.post(self.endpoint, data=xmlRequest,
                                       headers=constants.headers,
                                       proxies=proxyDictionary)
except Exception as httpException:
    anetLogger.error('Error retrieving http response from: %s ...')
    anetLogger.error('Exception: %s, %s', type(httpException), httpException.args)

if self._httpResponse:
    ...
else:
    anetLogger.debug("Did not receive http response")
return

requests.Response.__bool__ returns self.ok, i.e. status_code < 400. So a
404, 500 or 503 response is falsy, takes the else branch, and the
Response object — status code, headers and body — is thrown away.

Why it matters

Both failure modes converge on the same observable state:

what happened what the caller sees
connection refused / DNS failure / timeout getresponse() is None
HTTP 404 or 503 from an edge/load balancer getresponse() is None
success parsed response

There is no way to tell "the network is down" from "the API answered 503" from
"a proxy returned an HTML error page". Both diagnostic paths are also invisible
in a default install: the exception path logs to anetLogger, which the SDK
attaches a NullHandler to, and the HTTP-error path logs at DEBUG.

This is not theoretical. In a production integration we traced a period where
roughly 10% of API calls were being answered with an instant, empty-bodied
HTTP 404 by one of the API's own front-end nodes. From inside the SDK this was
indistinguishable from a transient network fault, and there was no log line at
all at default settings — the only evidence anywhere was a business-level
symptom (charges that succeeded at the gateway but whose follow-up lookup came
back empty). Diagnosing it required monkey-patching requests inside the SDK
to see the status codes.

Suggested fix

Test the response explicitly rather than relying on truthiness, and preserve
what came back so callers can act on it:

if self._httpResponse is not None and self._httpResponse.status_code < 400:
    ...  # existing success path
elif self._httpResponse is not None:
    anetLogger.error(
        'HTTP %s from %s (body: %.200r)',
        self._httpResponse.status_code, self.endpoint, self._httpResponse.text)
else:
    anetLogger.error('No HTTP response from %s', self.endpoint)

Even without an API change, raising the two failure logs from DEBUG/
swallowed to ERROR — and keeping the status code somewhere callers can read
it — would make this class of fault diagnosable instead of invisible.

Happy to open a PR if the shape above looks right.

Version

Observed on 1.1.4; the code is unchanged on current master.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions