Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timeout errors when refreshing tokens #614

Closed
NelsonFrancisco opened this issue Nov 17, 2022 · 6 comments
Closed

Timeout errors when refreshing tokens #614

NelsonFrancisco opened this issue Nov 17, 2022 · 6 comments

Comments

@NelsonFrancisco
Copy link

SDK you're using

  • Version 4.20.1
  • openid-client version 4.9.1

Describe the bug
I have an application that manages integrations with several Xero organisations and, hourly, calls Client.refreshToken hundreds of times (one time per organisation) within an interval of 1 or 2 seconds.
We've been getting a lot of timeouts lately. All of them to 3500ms

Here's the error in detail:

{"name":"TimeoutError","code":"ETIMEDOUT","timings":{"start":1668661558613,"socket":1668661558613,"lookup":1668661558660,"connect":1668661558681,"secureConnect":1668661558686,"upload":1668661558687,"error":1668661566384,"phases":{"wait":0,"dns":47,"tcp":21,"tls":5,"request":1,"total":7771}},"event":"request"}
Timeout awaiting 'request' for 3500ms

Please keep in mind that the error does not trigger for every request. Only for some.

I've already contacted Xero's support, and they told me to open a Issue in Github. So here it is.
There is a history already in the emails, so I'll try to sum it up:

  1. I gave details of a specific timeout (Client ID, refresh token last 4 chars, timestamp) and they told me that the refresh token actually refreshed server side and returned a new token. We did not store this new refresh token on our side because the client (the call to Client.refreshToken) timed out after 3500ms
  2. I replied with source code information:

It looks like the server actually renews the token, but the client does not receive the new token because it times out.

Like I previously said, we are using "xero-node" and calling the "XeroClient.refreshToken" method (here:

public async refreshToken(): Promise<TokenSet> {
). This method then calls openid Client.refreshToken (https://github.com/panva/node-openid-client/blob/1e3892e6222fdd1956735f97584ebc722fcebdd3/docs/README.md#clientrefreshrefreshtoken-extras).

openid Client has the option to set the timeout for the requests. The default is 3500ms. And this is the time that is being used by XeroClient when using the openid Client.
When you initialize the client (here:

custom.setHttpOptionsDefaults({
) the configuration "httpTimeout" is only for the retries.

And, as said before, these 3500ms are also being observed in several of our logs when trying to refresh the token.

So, we came to the conclusion that yes, the token is being refreshed, but we are not getting it because the "xero-node" library does not allow us to, because the client times out faster than the server can respond, not fetching the new refresh token value.

So, assuming this is true, can your servers take more than 3500ms to answer the refresh token request?

Is there any way around this situation?

To Reproduce
Everytime we need to refresh token for 200+ organisations, we get these errors.

Expected behavior
The calls to XeroClient.refreshToken not to timeout and return the new token

@RettBehrens
Copy link
Contributor

Hi @NelsonFrancisco taking a look at this and my proposed solution is to update XeroClient to allow for configurable timeout in addition to retry. Something like this:

custom.setHttpOptionsDefaults({
  retry: {
    maxRetryAfter: this.config.httpTimeout || 3500
  },
  timeout: this.config.httpTimeout || 3500
})

Thoughts? Would this resolve your issue?

@NelsonFrancisco
Copy link
Author

It might solve the issue indeed.
I actually have thought about that solution already.
Would it be a good PR?

@daffron
Copy link
Contributor

daffron commented Nov 28, 2022

Im also now getting this error, so the timeout in this SDK is non-configurable?

@RettBehrens
Copy link
Contributor

Hey all. Just catching up after the Thanksgiving holiday. @NelsonFrancisco if you want to submit a PR for that we'd welcome your contribution.

@daffron
Copy link
Contributor

daffron commented Dec 2, 2022

Hey, Not sure if you where going to do it or not @NelsonFrancisco so I have just quickly put #616 up, hope you don't mind/ hadn't started!

@RettBehrens
Copy link
Contributor

https://github.com/XeroAPI/xero-node/releases/tag/4.31.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants