2.0.5 is generating multiple refresh requests per refresh cycle

[GaryAustin1]

Bug report

Describe the bug

Supabase-js 2.0.5 is generating 2 refresh requests for every refresh timer cycle. This is with supabase-js, javascript only.
Reverting to 2.0.4 goes back to one request.

Another user is seeing similar multiple refreshes with auth-helpers in nextjs with 2.0.5 and getting refresh token reuse errors after awhile. Discord discussion 1/2 way and below of user Karbust's related issue: https://discord.com/channels/839993398554656828/1038545959640117289

To Reproduce

Run supabase-js with a logged in user and onAuthStateChange (optional as network tab shows same issue).

Expected behavior

One refresh request per gotrue-js refresh cycle.

Screenshots

2.0.5 console.logs with 60 second jwt expire. (note 120 seconds does same thing)

2.0.4 console.logs

2.0.5 network tab:

System information

Supabase-js 2.0.5
Multiple browsers
Javascript standalone and Nextjs with auth-helpers.

Additional context

Add any other context about the problem here.

[Karbust]

Getting 4 requests each time with @supabase/supabase-js 2.0.5 and @supabase/auth-helpers-nextjs 0.5.1. Tried downgrading to 2.0.4 and 0.4.5, respectively, but same issue.

[j4w8n]

Believe I found the issue. Was introduced with gotrue-js v2.2.2 - which is the version required in supabase-js v2.0.5. I'm just not sure how the maintainers want to fix it (rollback a change, or re-write the code to work differently). PR #482

Tagging a few peeps, as a heads-up and to double-check my theory. @hf @david-plugge @thorwebdev

Lets say you've set your JWT to expire in 240 seconds.
Here is the flow:

1. User logs in and _saveSession is called.
2. Inside _saveSession, on line 996, _startAutoRefreshToken is called; and (almost) always passes in a number which triggers a token refresh attempt when your JWT has 10 seconds until it expires. So in our example, it would pass in 230 since that's 10 seconds before the original 240 is up.
3. Once triggered, the refresh code first calls getSession(). It's doing this so it can grab the session's refresh_token, in order to use it to refresh the access_token. But here is where things go wrong, because of some recently updated code at cf0d32c

In the getSession() code, linked to in number 3 above, it checks if the session is expired. Because of the code change, this will always equal true when the client tries to auto refresh the token. Let me explain with an example:

There are three things as part of the hasExpired calculation. session.expires_at, timeNow and EXPIRY_MARGIN.
Lets say expires_at is 3000, that would make timeNow ~2990 (10 seconds till expiry; but may be higher than 2990), and EXPIRY_MARGIN is 10 per definition. So, the code is asking the question, "is 3000 less-than or equal-to 2990+10?". This will always be true in our case.

4. If hasExpired equals true, getSession() will refresh the access token. That's one refresh.
5. Then getSession returns the session to the _startAutoRefreshToken code; which says, "oh, we have a session? let's refresh the token." That's your second refresh.

[j4w8n]

If PR #482 is helping to resolve other issues, and is desirable to keep, then one solution to the current issue is to only call getSession() and be done.

  private _startAutoRefreshToken(value: number) {
    if (this.refreshTokenTimer) clearTimeout(this.refreshTokenTimer)
    if (value <= 0 || !this.autoRefreshToken) return

    this.refreshTokenTimer = setTimeout(async () => {
      this.networkRetries++
      const {
        data: { session },
        error
      } = await this.getSession()
      if (!error && session) {
        this.networkRetries = 0
      }
      if (
        error instanceof AuthRetryableFetchError &&
        this.networkRetries < NETWORK_FAILURE.MAX_RETRIES
      )
        this._startAutoRefreshToken(NETWORK_FAILURE.RETRY_INTERVAL ** this.networkRetries * 100) // exponential backoff
    }, value)
    if (typeof this.refreshTokenTimer.unref === 'function') this.refreshTokenTimer.unref()
  }

Rather than the current code:

  private _startAutoRefreshToken(value: number) {
    if (this.refreshTokenTimer) clearTimeout(this.refreshTokenTimer)
    if (value <= 0 || !this.autoRefreshToken) return

    this.refreshTokenTimer = setTimeout(async () => {
      this.networkRetries++
      const {
        data: { session },
        error: sessionError,
      } = await this.getSession()
      if (!sessionError && session) {
        const { error } = await this._callRefreshToken(session.refresh_token)
        if (!error) this.networkRetries = 0
        if (
          error instanceof AuthRetryableFetchError &&
          this.networkRetries < NETWORK_FAILURE.MAX_RETRIES
        )
          this._startAutoRefreshToken(NETWORK_FAILURE.RETRY_INTERVAL ** this.networkRetries * 100) // exponential backoff
      }
    }, value)
    if (typeof this.refreshTokenTimer.unref === 'function') this.refreshTokenTimer.unref()
  }

[hf]

This is probably going to get resolved with #564.

[j4w8n]

I believe this already got resolved. The ticket has just remained open.