snowplow-javascript-tracker icon indicating copy to clipboard operation
snowplow-javascript-tracker copied to clipboard

Retry is disabled by default for `POST` method

Open mmiermans opened this issue 4 months ago • 2 comments

Describe the bug Retries are disabled by default for the POST method. We used a config similar to the one below, intending to have 3 retries:

gotEmitter(
  process.env.SNOWPLOW_ENDPOINT, // endpoint
  'https',
  undefined, // port
  'post', // method
  1, // buffer size
  3, // retries
);

However, Snowplow events were not retried on failure due to the Got defaults:

By default, Got does not retry on POST.

To Reproduce Configure Snowplow with 'post' method and retries >= 1.

Expected behavior If Snowplow is configured with a positive number of retries, then emit should be retried on network error.

Suggested solution By default, enable retries for GET and POST methods in gotEmitter.

If the retry argument is a number (instead of an object), then the following object is passed to Got:

const retries: Partial<RequiredRetryOptions> = {
  limit: retry,
  methods: ['GET', 'POST'],
};

Otherwise, if the retry.methods is undefined it should default to be enabled for both methods used by Snowplow:

const retries: Partial<RequiredRetryOptions> = {
  methods: ['GET', 'POST'],
  ...retry,
};

Desktop (please complete the following information):

  • OS: Linux
  • Node: v20.11.0

Additional context @snowplow/node-tracker version 3.19.0

mmiermans avatar Mar 04 '24 22:03 mmiermans

Once we found out about this, a simple workaround is to set retry to:

{
  limit: 3,
  methods: ['GET', 'POST'],
};

However, many people (like us) might have simply passed in a number, and assumed that would work. We only found out about this once events seemed not to arrive, and we started investigating.

mmiermans avatar Mar 04 '24 22:03 mmiermans

Thanks for reporting this @mmiermans , that is a good catch!

matus-tomlein avatar Mar 07 '24 15:03 matus-tomlein