Node.js: Why Braintree methods resolve promise with error response?

Question

In Braintree Node.js SDK, the methods will return a promise if no callback is given. For example:

1st time

gateway.customer.create({
        firstName: "First",
        lastName: "Last",
        email: "test@abc.com",
        id: "123123"
    })
    .then((response) => {
        console.log(response); // correct response from this line
    })
    .catch((err) => {
        console.error(err);
    });

It will log the response correctly.

After that, I execute the same code again. I expect a fail because of duplicate custom ID.

2nd time

gateway.customer.create({
        firstName: "First",
        lastName: "Last",
        email: "test@abc.com",
        id: "123123"
    })
    .then((response) => {
        console.log(response); // error response from this line
    })
    .catch((err) => {
        console.error(err);
    });

It will log the error response in the then block.

3rd time

gateway.customer.find("1231234") // not existing
    .then((response) => {
        console.log(response);
    })
    .catch((err) => {
        console.error(err); // error from this line
    });

It will log the Not Found from catch block.

Is there any reason the 2nd give a resolve with an error while the 3rd give a reject? Is it safe if I write a promisifier and treat all response with success: false as reject?

Answer 1

This answer comes from github, which explain the issue perfectly. https://github.com/braintree/braintree_node/issues/130#issuecomment-370079910

Braintree's underlying API that the different server SDKs use follows a result object pattern.

So, if you have a transaction that succeeds, you get:

gateway.transaction.sale({
  amount: '5.00',
  paymentMethodNonce: 'fake-valid-nonce',
  options: {
    submitForSettlement: true
  }
}).then((result) => {
  // result.success === true
  // result.transaction is the transaction object
})

Now, lets say you have a transaction that is processor declined:

gateway.transaction.sale({
  // sandbox amount to trigger processor declined 
  // https://developers.braintreepayments.com/reference/general/testing/node#test-amounts
  amount: '2000.00', 
  paymentMethodNonce: 'fake-valid-nonce',
  options: {
    submitForSettlement: true
  }
}).then((result) => {
  // result.success === false
  // result.transaction is still the transaction object
})

In this case, even though the transaction failed, a transaction object is still created, so it makes sense that the promise resolves, but reports that the sale was not a success.

There are only a few cases where the promise will actually reject. One is if the Braintree Gateway cannot be reached (no internet, revoked/incorrect API keys, rate limiting):

braintree.transaction.sale(saleOptions).catch((err) => {
  // you'll get here because the Braintree gateway could not be reached
  // or the API keys are invalid
  // or there have been too many requests made, etc
});

Or if the resource could not be found:

gateway.transaction.find('bad-id').catch((err) => {
  // this will result in a not found error
});

In basically every other case, the promise will resolve with result.success being true or false.

This pattern works very well for transactions, where you need a record of the attempted transaction even if the transaction did not actually charge the customer. It works less well for the example you gave:

braintree.clientToken.generate({ customerId: 'invalid' }).then((res) => {
  // res.sucess === false
  // res.message === 'Customer specified by customer_id does not exist',
});

Where there's no underlying object being created.

I think we used this pattern largely because the other Braintree server SDKs used this pattern. And the reason the other SDKs used this pattern was because the alternative was to have the SDK raise an exception in the case where a transaction failed (because those languages don't have the same async patterns as node, so you literally have to throw an error if the request failed).

I agree that this pattern is not perfect, but right now we can't change it without both a major version bump and having a big inconsistency with the other server sdks.