prevent got from automatic retry on error - fix #26

[jasonblewis]

fix #26

The got library's default retry behaviour was causing unnecessary delays
and inconsistent error handling in the netsuite-api-client. This fix explicitly
disables retries by adding retry: { limit: 0 } to the request options.

This ensures that errors are handled immediately without additional delays.

[julbrs]

Wow, I wasn't aware that got was doing automatic retry by default. We have implemented a kind of automatic retry on our own code on top of this 😅

Thanks a lot for the issue and the PR, I will get a look shortly on that!

[jasonblewis]

Maybe the limit option should be added to the got client so all got calls use it? like this:

https://github.com/sindresorhus/got/blob/main/documentation/quick-start.md#options

import got from 'got';

const options = {
	prefixUrl: 'https://httpbin.org',
	headers: {
		Authorization: getTokenFromVault(),
       retry: {limit: 0},
	},
};

const client = got.extend(options);

export default client;

(I'm just learning node and js so please let me know if you think that is a good or bad idea?)

[jasonblewis]

retry on our own code on top of this 😅

I am doing the same to mitigate hitting concurrency limits. Thinking outside the box here but perhaps this kind of concurrency limit and retry could be moved into the client.js? as in I think it needs a bit more nuance than got's default retry method, but probably all users of this lib will need concurrency limits, so that might be a good place to put it?

[julbrs]

The project is using pnpm, so you can remove this package-lock.json ;)

[julbrs]

what is this commented line?

[julbrs]

what is the goal of the timeout parameter?

[julbrs]

i don't understand exactly what is the timings prop in NetSuiteOptions?

[julbrs]

I think it can be a good idea to have this stuff as an option of the client. I am a bit nervous about changing that ;D Do you think you can adjust the PR? thanks!

[jasonblewis]

Hi Julien, thanks for getting back to me.

Apologies for the messy PR, I'm going to withdraw it for now. Since I raised the PR I found that I was still getting these odd long 60s hangs before the got request would return. I still have not worked out how to resolve it but I think I understand what is going on.

When the request is sent via got, if it receives a 429 response, I believe got takes it upon itself to retry after whatever is in the retry-after response header. in NS's case it's 60, so the got request waits for 60s. when it finally does retry, it reuses the original authorization header it used when it got the 429, leading to the following 401 errors: message: 'Invalid login attempt. For more details, see the Login Audit Trail in the NetSuite UI at Setup > Users/Roles > User Management > View Login Audit Trail.',

Unfortunately I'm not expert (yet) in all the moving parts of js, typescript and nodejs and I'm learning as I go. This error could possibly be due to me incorrectly using your netsuite-api-client.

I think a good approach may be to hand off retries to got, but also hand it a method of regenerating the authorisation header.

If you want more details about how i diagnosed this, happy to share. I'm attempting to find a solution but as i said, right now it's eluding my rudimentary skills.

ps I have tried setting retry.limit = 0 in the options in client.ts but for some reason got is ignoring that. I'm still investigating.