New Axios interceptor is added to default Axios instance for each API Client instantiated

[better-ssallay]

My team just started using the SDK for our geocoding service and we ran into a weird bug. We were building a new API client for each request made, and we noticed that latency would slowly creep upwards over time.

At first, I thought it was some kind of issue with a preconfigured HTTP Keep Alive header, but when I looked at the SDK source, I saw that the HttpSender class calls axiosRetryon the default Axios export every time the constructor is called. After a few thousand requests, we would have a few thousand interceptors attached that would run during every request.

Here's a minimal recreation of the issue:

import axios from 'axios-proxy-fix';
import * as SmartyStreetsSDK from 'smartystreets-javascript-sdk';

const clientBuilder = new SmartyStreetsSDK.core.ClientBuilder(
  new SmartyStreetsSDK.core.StaticCredentials('abc-123', 'xyz-456')
);

let originalInterceptorCount = 0;

axios.interceptors.request.forEach(() => {
  originalInterceptorCount++;
});

clientBuilder.buildUsStreetApiClient();
clientBuilder.buildInternationalStreetClient();
clientBuilder.buildUsAutocompleteClient();

let afterBuildInterceptorCount = 0;

axios.interceptors.request.forEach(() => {
  afterBuildInterceptorCount++;
});

console.log(originalInterceptorCount);       // Prints 0
console.log(afterBuildInterceptorCount);   // Prints 3

Admittedly, we should not have been creating a new client for every request. We have already updated our code to only instantiate one client. However, this seems like weird behavior that could create issues for users that are using more than one type of client.

In my mind, the ideal solution would be to call axios.create to instantiate a separate axios instance for each instance of HttpSender.

[tiosalazar]

The reason is cuz the last version of https://www.npmjs.com/package/axios-retry https://github.com/softonic/axios-retry "axios-retry": "3.2.2", damage that,

you should install the next version to fix that issue.

"axios-retry": "3.2.0",

[camiblanch]

This may have been fixed with #38 and version 1.11.2 where I upgraded the axios-retry package version to ^3.2.3. Let me know if you are still seeing this issue after installing the latest version of the SDK

[camiblanch]

I have actually reverted the change to bring axios-retry down to 3.2.0 because 3.2.3 was giving some new undefined issues. You will want to grab version 1.11.3 of the SDK for these changes

[ehaynes99]

If you're attaching listeners or wiring up axios-retry, you should create a dedicated instance rather than using the global one. Example:

class HttpSender {
  constructor(timeout = 10000, retries = 5, proxyConfig, debug = false) {
    this.axiosInstance = Axios.create();
    axiosRetry(axiosInstance, {
      retries: retries,
    });
    // ...
  }

  // ...

  send(request) {
    return new Promise((resolve, reject) => {
      let requestConfig = this.buildRequestConfig(request);

      // All calls currently made on `Axios` are instead called on the instance
      this.axiosInstance(requestConfig).then(response => {
        // ...

As it is now, you're modifying the behavior of axios across the entire application. Since it's using axios-proxy-fix instead of the normal axios, there is less of a chance that consuming apps are using the same global instance, but that's definitely not guaranteed. E.g. if debug is true, you'll start console logging ALL requests made to ANY service, not just Smarty Streets. This has both security and performance implications. Example:

const Axios = require('axios-proxy-fix');
const SmartyStreetsSDK = require('smartystreets-javascript-sdk');

const authId = process.env.SMARTY_STREETS_ID;
const authToken = process.env.SMARTY_STREETS_TOKEN;

const run = async () => {
  const smartyStreets = new SmartyStreetsSDK.core.ClientBuilder(
    new SmartyStreetsSDK.core.StaticCredentials(authId, authToken),
  )
    .withDebug()
    .buildUsStreetApiClient();

  const lookup = Object.assign(new SmartyStreetsSDK.usStreet.Lookup(), {
    street: '1600 Pennsylvania Avenue NW',
    city: 'Washington',
    state: 'DC',
    zipCode: '20500',
  });

  await smartyStreets.send(lookup);

  // auth headers are now logged!
  const response = await Axios.get('https://jsonplaceholder.typicode.com/todos/1', {
    headers: { Authorization: 'Bearer super-secret-token' },
  });
  console.log('***** response:', response.data);
};

run();