1.) Fix crashes while attempting to log exceptions.
The log.warn() and log.exceptions() calls need a '%s' to render properly, but an additional '%' is needed to tell the % operator to not expand the '%s' until the logging method is called.
2.) Remove special logic that stops building the response classes upon errors.
Below is an explanation of how this library behaves with the check in place and with the check removed. The case below shows why the check should be removed: because clients may want to inspect additional information provided from the server and not consume it conditionally from two places.
Before removing the response.result != 0 check:
When a payment was submitted and failed to succeed, an error code was returned. The field 'avsaddr' was then placed in unconsumed_data instead of in an AddressVerificationResponse object.
It is reasonable to want to inspect additional data, such as 'avsaddr' even in failure conditions, and special logic must be used to either check AddressVerificationResponse object or unconsumed_data, depending on the situation. This makes client code using this library inconsistent.
Before removing the response.result != 0 check:
With or without a successful transaction, objects such as AddressVerificationResponse are constructed as long as the server provides fields.
Now, we only need to check for the 'avsaddr' field in one place (the AddressVerificationResponse), never in unconsumed_data.
Testing:
Tested extensively with Python 2.7
Verified fixes work correctly in sandbox environment.
This pull request fixes two things:
1.) Fix crashes while attempting to log exceptions.
The log.warn() and log.exceptions() calls need a '%s' to render properly, but an additional '%' is needed to tell the % operator to not expand the '%s' until the logging method is called.
2.) Remove special logic that stops building the response classes upon errors.
Below is an explanation of how this library behaves with the check in place and with the check removed. The case below shows why the check should be removed: because clients may want to inspect additional information provided from the server and not consume it conditionally from two places.
Before removing the response.result != 0 check: When a payment was submitted and failed to succeed, an error code was returned. The field 'avsaddr' was then placed in unconsumed_data instead of in an AddressVerificationResponse object. It is reasonable to want to inspect additional data, such as 'avsaddr' even in failure conditions, and special logic must be used to either check AddressVerificationResponse object or unconsumed_data, depending on the situation. This makes client code using this library inconsistent.
Before removing the response.result != 0 check: With or without a successful transaction, objects such as AddressVerificationResponse are constructed as long as the server provides fields. Now, we only need to check for the 'avsaddr' field in one place (the AddressVerificationResponse), never in unconsumed_data.
Testing: