SwedbankPay / swedbank-pay-sdk-dotnet

Swedbank Pay SDK for .NET (Beta)
https://www.nuget.org/packages/SwedbankPay.Sdk
Apache License 2.0
7 stars 19 forks source link

Improve API Problem handling #98

Closed SteffenSH closed 3 years ago

SteffenSH commented 4 years ago

This issue is about how to identify Problems – which is neccessary for correct handling in client implementations (e.g. when a payment fails beacuase of insufficient funds etc).

I'm posting this as a new issue here, but it started off as a conversation in this pull request: https://github.com/SwedbankPay/swedbank-pay-sdk-dotnet/pull/94

I think Swedbank have made a good choice by transitioning over to RFC7807 for error reporting. But the way you currently have implemented and documented it makes it hard for us to use on the client-side. E.g. this JSON response:

{
  "transactionList": [
    {
      "failedReason": "ExternalResponseError",
      "failedActivityName": "Authorize",
      "failedErrorCode": "REJECTED_BY_ACQUIRER_CARD_STOLEN",
      "failedErrorDescription": "card stolen, response-code: 43",
      "isOperational": false,
      "problem": {
        "type": "https://api.payex.com/psp/errordetail/creditcard/acquirercardstolen",
        "title": "Operation failed",
        "status": 403,
        "detail": "Unable to complete Authorization transaction, look at problem node!",
        "problems": [
          {
            "name": "ExternalResponse",
            "description": "REJECTED_BY_ACQUIRER_CARD_STOLEN-card stolen, response-code: 43"
          }
        ]
      }
    }
  ]
}

The type URI https://api.payex.com/psp/errordetail/creditcard/acquirercardstolen do not dereference to an HTML explaining the problem. I found this documentation though for some legacy error codes: https://developer.swedbankpay.com/resources/test-data#amount-error-testing-method But I could not find anything in the documentation about how these URIs (types) relate to those legacy error codes. Also RFC7807 states that:

"detail" (string) - A human-readable explanation specific to this occurrence of the problem.

But in the API we get this response back: "detail": "Unable to complete Authorization transaction, look at problem node!", This is hardly human readable.

Also: The problems property contains an array of error objects. How should we interpret these? E.g. can there be multiple errors? Which of them should we report back to the users/customers? The description property looks like a concatenation of the old failedErrorCode and failedErrorDescription: "description": "REJECTED_BY_ACQUIRER_CARD_STOLEN-card stolen, response-code: 43" Maybe this instead could be split up into multiple extension members so that it's less complex for the clients to interpret?

In summary what I really need is:

  1. A list of the most common type URI's that we can expect. E.g. something similar to the list of legacy error code (https://developer.swedbankpay.com/resources/test-data#amount-error-testing-method).
  2. An easy way to find a human readable description of the error to display to the customers (on web or email).
  3. An implementation of these problem properties in the .NET SDK. You have already invited me to implement this myself and then do a pull request again. But it's a bit hard for me to do because you have supplied no documentation for the extension members you will be using. And it's also a bit hard to explain to our employer that we are spending time implementing an SDK that is owned by Swedbank.
asbjornu commented 4 years ago

Thanks for the great feedback, @SteffenSH.

The type URI https://api.payex.com/psp/errordetail/creditcard/acquirercardstolen do not dereference to an HTML explaining the problem.

Correct. type is currently just used as an identifier. We plan on making that identifier dereferenceable, though.

But I could not find anything in the documentation about how these URIs (types) relate to those legacy error codes.

You should be able to find some in the "Other Features" page of each section, see Card Problems as an example.

"detail" (string) - A human-readable explanation specific to this occurrence of the problem.

But in the API we get this response back: "detail": "Unable to complete Authorization transaction, look at problem node!", This is hardly human readable.

While I agree it could be more descriptive, I insist that Unable to complete Authorization transaction, look at problem node! is a human-readable string and not meant for machine consumption.

Sometimes we surface errors from third parties that are hard for us to provide user friendly descriptions of. In such cases, the detail and especially description may be less user friendly. We are sorry about that, but error handling is a continuous effort of improvement we are working on.

Also: The problems property contains an array of error objects. How should we interpret these?

The problems property is most useful when the problem+json is for a 400 Bad Request response given as a validation error. Then each name should correspond to a property in the invalid request, which should be possible to correlate in the UI.

E.g. can there be multiple errors?

Yes.

Which of them should we report back to the users/customers?

That depends on your UI, but I would mostly correlate this to the HTTP status code and type. For requests coming from the end user, you should be able to present most of what you find in an application/problem+json response back to the end user.

"description": "REJECTED_BY_ACQUIRER_CARD_STOLEN-card stolen, response-code: 43" Maybe this instead could be split up into multiple extension members so that it's less complex for the clients to interpret?

No, the point is that the status code and type should be all you need to check programmatically to know what to do with the error. If that doesn't give you enough information, please let us know and we'll add more type URIs.

  1. A list of the most common type URI's that we can expect. E.g. something similar to the list of legacy error code (https://developer.swedbankpay.com/resources/test-data#amount-error-testing-method).

  2. An easy way to find a human readable description of the error to display to the customers (on web or email).

I hope I have answered these above.

  1. An implementation of these problem properties in the .NET SDK. You have already invited me to implement this myself and then do a pull request again. But it's a bit hard for me to do because you have supplied no documentation for the extension members you will be using. And it's also a bit hard to explain to our employer that we are spending time implementing an SDK that is owned by Swedbank.

Completely understood. You are of course free to wait until we have implemented this, I just can't give you any guaranteed timeframe of when it will be done.

SteffenSH commented 4 years ago

Thanks @asbjornu for good and thorough answers to my questions and for responding so quickly to this! The documentation you pointed me to helps a lot of course and I think this answer most of my questions.

Though it would be nice to have a human and customer readable detail field that we could always use for display on web and email. I don't think there is such a field directly available now. But anyways… The documentation you pointed me to lists a lot of these type identifiers together with descriptions – so we could probably just put these somewhere in our own system (client side) and maintain a type-to-customerReadableError mapping there. And for the ones we are omitting, we will just display a generic error message to the customer.

PS. I will leave this issue open until either of us has implemented the Problem properties in the .NET SDK.

asbjornu commented 3 years ago

Can you please try out version 2 of the SDK and let us know whether it resolves this issue, @SteffenSH?

asbjornu commented 3 years ago

Closing as resolved. Please reopen if there's still stuff to do here.