goshippo / shippo-node-client

Shipping API Node.js library (USPS, FedEx, UPS and more)
https://goshippo.com/docs
MIT License
136 stars 54 forks source link

Errors not captured properly handling 4XX or 5XX response codes #91

Open jbool24 opened 1 year ago

jbool24 commented 1 year ago

When using the debugger during a call with shippo.transaction.create({{PAYLOAD}}), the request seems to fire twice. The first time with a valid payload describing the response, and the second payload only contains { details: 'Permission Denied' }. This breaks a previous contract that says there will be a status field on the response object.

This code is contained in Method.js on line 79.

To recreate, simply add a breakpoint on this line and use the function call shippo.transactions.create() method to see the request fire twice.

jbool24 commented 1 year ago

And the example output from watch in JS debugger: first time through:

response: {
  carrier_accounts: [ ],
  object_created: "2023-05-31T21:28:45.481Z",
  object_updated: "2023-05-31T21:28:45.563Z",
  object_id: "{{REDACTED}}",
  object_owner: "{{REDACTED}}",
  status: "SUCCESS",
  address_from: {
    object_id: "{{REDACTED}}",
    is_complete: true,
    name: "{{REDACTED}}",
    company: "{{REDACTED}}",
    street_no: "",
    street1: "{{REDACTED}}",
    validation_results: { },
    street2: "",
    street3: "",
    city: "{{REDACTED}}",
    state: "{{REDACTED}}",
    zip: "{{REDACTED}}",
    country: "US",
    phone: "{{REDACTED}}",
    email: "",
    is_residential: null,
    test: true,
  },
  address_to: {
    object_id: "{{REDACTED}}",
    is_complete: true,
    name: "{{REDACTED}}",
    company: "",
    street_no: "",
    street1: "{{REDACTED}}",
    validation_results: { },
    street2: "",
    street3: "",
    city: "{{REDACTED}}",
    state: "{{REDACTED}}",
    zip: "{{REDACTED}}",
    country: "US",
    phone: "{{REDACTED}}",
    email: "",
    is_residential: null,
    test: true,
  },
  parcels: [
    {
      object_state: "VALID",
      object_created: "2023-05-31T21:28:45.450Z",
      object_updated: "2023-05-31T21:28:45.501Z",
      object_id: "{{REDACTED}}",
      object_owner: "{{REDACTED}}",
      template: null,
      extra: {},
      length: "12.0000",
      width: "8.0000",
      height: "1.0000",
      distance_unit: "in",
      weight: "2.0000",
      mass_unit: "oz",
      value_amount: null,
      value_currency: null,
      metadata: "",
      line_items: [ ],
      test: true,
    },
  ],
  shipment_date: "2023-05-31T21:28:45.449Z",
  address_return: {
    object_id: "{{REDACTED}}",
    is_complete: true,
    name: "{{REDACTED}}",
    company: "{{REDACTED}}",
    street_no: "",
    street1: "{{REDACTED}}",
    validation_results: { },
    street2: "",
    street3: "",
    city: "{{REDACTED}}",
    state: "{{REDACTED}}",
    zip: "{{REDACTED}}",
    country: "{{REDACTED}}",
    phone: "{{REDACTED}}",
    email: "",
    is_residential: null,
    test: true,
  },
  alternate_address_to: null,
  customs_declaration: null,
  extra: {
    bypass_address_validation: false,
  },
  rates: [
  ],
  messages: [
  ],
  metadata: "{{REDACTED}}",
  test: true,
  order: null,
}

second time through response: { detail: "Permission denied" }

jbool24 commented 1 year ago

Looks like line 118 of Resource.js parses var response value as JSON but disregards the status code 403 so it's not instantiated as a custom shippo error here

_responseHandler: function(req, callback) {
        var self = this;
        return function(res) {
            // console.log('status %s', res.statusCode);
            var response = '';

            res.setEncoding('utf8');
            res.on('data', function(chunk) {
                response += chunk;
            });
            res.on('end', function() {
                var err;

                var errData = {
                    detail: response,
                    path: req.path,
                    statusCode: res.statusCode
                };

                try {
                    response = JSON.parse(response);
                } catch (e) {
                    errData.message = "Invalid JSON received from the Shippo API";
                    return callback.call(
                        self,
                        new Error.ShippoAPIError(errData),
                        null
                    );
                }

  /***********************************************
   * NO 403 Handler below (or other codes)
   * resulting error is misshaped
   * which consumers' downstream calls are unable to check for an error response
   ***********************************************/
                if (res.statusCode === 401) {
                    errData.message = "Invalid credentials";
                    err = new Error.ShippoAuthenticationError(errData);
                } else if (res.statusCode === 404) {
                    errData.message = "Item not found";
                    err = new Error.ShippoNotFoundError(errData);
                } else if (res.statusCode === 301) {
                    errData.message = "API sent us a 301 redirect, stopping call. Please contact our tech team and provide them with the operation that caused this error.";
                    err = new Error.ShippoAPIError(errData);
                } else if (res.statusCode === 400) {
                    errData.message = "The data you sent was not accepted as valid";
                    err = new Error.ShippoAPIError(errData);
                }
                if (err) {
                    return callback.call(self, err, null);
                } else {
//  403 and 500 errors fall through as a success response here because 'err' is still undefined after if-else flow
                    callback.call(self, null, response);
                }
            });
        };
    },