next-auth icon indicating copy to clipboard operation
next-auth copied to clipboard

Published 20 hours ago • verified publisher icon nextauthjs

Allow pausing the session polling when the browser is offline

Open jozefhruska opened this issue 2 years ago • 10 comments

Description 📓

When the session polling is enabled (refetchInterval option), the request to /session is made regardless of the browser networks status. If the browser is offline (i.e. computer puts itself to sleep), the request fails, and the session is invalidated (null). This behavior can cause UI issues and make other requests fail when the application stores access tokens in the session object.

Example (next-auth + SWR):

  1. The app is mounted, and SWR fetches data successfully.
  2. The host device goes offline => /session request fails => session is invalidated (null).
  3. The host device goes back online, triggering the window focus event.
  4. SWR tries to fetch the data but sends unauthenticated requests because the session is null, resulting in an error.

Suggested solution:

We could add a new flag similar to the SWR's refreshWhenOffline. When disabled, all requests would be paused while the browser is offline, so the session would not be invalidated. This flag wouldn't introduce any breaking change, but it'd make the behavior configurable by the user.

The issue is related to #2215, but the stale bot didn't reopen the issue when I commented on it. As an alternative to this suggestion, we could stop the session from being invalidated, but I agree with the point made in https://github.com/nextauthjs/next-auth/issues/2215#issuecomment-867190636.

How to reproduce ☕️

  1. Run the example app with at least one configured provider (Auth0, for example).
  2. Ensure the session polling is enabled (refetchInterval).
  3. Sign in with the previously configured auth provider.
  4. Use the dev tools to set the throttling to "Offline".
  5. Wait until the application tries to refetch the session.
  6. See the session gets invalidated.

Contributing 🙌🏽

Yes, I am willing to help implement this feature in a PR

jozefhruska avatar Jul 14 '22 16:07 jozefhruska

So I don't have a definitive answer, but as far as I am aware, setInterval (which refetchInterval uses) stops when the computer goes to sleep and resumes when it's back: https://stackoverflow.com/a/6347336/5364135

Answer here also suggests that inactive tabs have lower priority for executing JS so I would assume the same as above: https://stackoverflow.com/a/5927432/5364135

balazsorban44 avatar Jul 14 '22 23:07 balazsorban44

On a second read, I fixated a bit much on the "sleeping" part, when in reality you are discussing online/offline states here.

I think you make a good point. There is not much value in polling when there is no internet access. I'm opening a PR to make this opt-in, we might even want this as the default behavior in the next major release. :+1:

balazsorban44 avatar Jul 14 '22 23:07 balazsorban44

Thank you for such a swift implementation! Let me know if you need help with anything 🙂

jozefhruska avatar Jul 15 '22 07:07 jozefhruska

Yeah, you could try it out, here is an experimental published version https://github.com/nextauthjs/next-auth/pull/4940#issuecomment-1185042331

balazsorban44 avatar Jul 15 '22 10:07 balazsorban44

Okay, I tested it on my project. The session polling is indeed paused when the refreshWhenOffline flag is set to false. However, the session is still being refreshed on window focus causing it to be invalidated anyway.

I'm wondering whether the refreshWhenOffline flag should override the refetchOnWindowFocus flag or not. It's not exactly obvious which flag has the higher priority.

Sorry, I know this is not the original issue, but I wouldn't want this change to create confusion.

jozefhruska avatar Jul 15 '22 11:07 jozefhruska

Hmm, yeah. Good point and sounds relevant, but these are separate flags IMO, not necessarily that one has priority over the other.

You can already disable refetching on window focus. You could set refetchOnWindowFocus={typeof navigator !== "undefined" && navigator.onLine} (Extra check is added since navigator would not be available when prerendering the page at build-time)

~Maybe refreshWhenOffline -> refetchWhenOffline naming would be better and could cover both at the same time. But refresh* sounds like it should only change behavior when polling is enabled.~ I did call it that, so maybe we should just incorporate your suggestion. :sweat_smile: Although there is some ambiguity around refetch and refresh that might cause confusion. :thinking:

Not sure...

Let's call it refreshWhenOffline and suggest refetchOnWindowFocus={typeof navigator !== "undefined" && navigator.onLine} in addition in the docs to keep these separate?

(In the long run, we might want to revisit the efforts like https://github.com/nextauthjs/react-query to make the whole session state handling pluggable into other libs. At this point, we are recreating SWR and react-query. :shrug:)

balazsorban44 avatar Jul 15 '22 13:07 balazsorban44

Let's call it refreshWhenOffline and suggest refetchOnWindowFocus={typeof navigator !== "undefined" && navigator.onLine} in addition in the docs to keep these separate?

So if I understand correctly, there would be two options affecting the polling configuration?

  • refreshWhenOffline
  • refetchInterval

I'm afraid having different prefixes for these two might backfire quickly. I like how SWR distinguishes between options affecting the polling configuration (refresh* prefix) and one-time revalidation requests (revalidate* prefix). But that brings us back to your point about recreating SWR.

It seems like the only way to avoid confusion is to make the refetchWhenOffline also affect the revalidation on window focus. I can't figure out any other way which wouldn't introduce breaking changes 😞

(In the long run, we might want to revisit the efforts like https://github.com/nextauthjs/react-query to make the whole session state handling pluggable into other libs. At this point, we are recreating SWR and react-query. 🤷)

Not gonna lie, this sounds like it'd solve many issues. I don't think the built-in useSession will ever be so robust as SWR or react-query. If the only problem is you not having the bandwidth for maintaining another package, I can put together some up-to-date versions, at least for SWR and react-query.

jozefhruska avatar Jul 15 '22 16:07 jozefhruska

Partially the bandwidth/maintenance burden, but properly managing the session state would need to involve signIn and signOut as well, as those can mutate the state as well. Currently, it is a bit hacky, as these leave outside the React world:

https://github.com/nextauthjs/next-auth/blob/c56abbd74504687134732474425d5b99ae2a8ed3/packages/next-auth/src/react/index.tsx#L243

https://github.com/nextauthjs/next-auth/blob/c56abbd74504687134732474425d5b99ae2a8ed3/packages/next-auth/src/react/index.tsx#L291

Basically, we would need to recreate the whole next-auth/react module for each library. Or a better solution is to abstract away the session management and signIn/signOut methods that aren't tied to React at all but could be plugged into it.

This might also make it easier to create clients for other frameworks as we expand (#3942) and will probably be the next logical step.

To keep this feature request small though we could recommend this today:

<SessionProvider
  refetchInterval={60}
  refetchOnWindowFocus={typeof navigator !== "undefined" && navigator.onLine}
  refetchWhenOffline={false}
>

So this would poll the session every 60 seconds, but stop annulling the session when offline and would only refetch on window focus if you are online again.

balazsorban44 avatar Jul 15 '22 16:07 balazsorban44

Basically, we would need to recreate the whole next-auth/react module for each library. Or a better solution is to abstract away the session management and signIn/signOut methods that aren't tied to React at all but could be plugged into it.

Thanks for the explanation. I can see the complexity now.

So this would poll the session every 60 seconds, but stop annulling the session when offline and would only refetch on window focus if you are online again.

This seems to be an optimal solution for now. Thank you!

Edit: If anyone using SWR stumbles across this issue, I've created an SWR-powered alternative to @next-auth/react-query: https://github.com/jozefhruska/next-auth-swr

jozefhruska avatar Jul 17 '22 09:07 jozefhruska

As an alternative to this suggestion, we could stop the session from being invalidated, but I agree with the point made in #2215 (comment).

I was looking for a way to stop the session from being invalidated in a somewhat similar case, because it's kind of a problem, so I stumbled upon this issue. Preventing the invalidation (perhaps with a callback that receives the error and the previous session, and returns a session or null) would solve the problem for me as well.

In my case, I'm getting an access token from the session. Requests with this access token go to an external API and the token lives long enough so that one failed session call will never result in an expired token. If the session wasn't invalidated when getSession fails, the user would not notice that there was an error. With the current implementation however, the user is logged out.

kmsomebody avatar Sep 20 '22 00:09 kmsomebody 

<SessionProvider
  refetchInterval={60}
  refetchOnWindowFocus={typeof navigator !== "undefined" && navigator.onLine}
  refetchWhenOffline={false}
>

@ThangHuuVu @balazsorban44 hi guys! I tried this but it is not working. Session is still being refreshed even if user is offline

PS: the only thing which i did not configure was refetchInterval={60} as I don't want the network cost due to a call to session being made every 60secs (I have kept it as default i.e. 0)

nik32 avatar Jun 25 '23 14:06 nik32

@jozefhruska is this feature still working for you? I am using NextAuth - 4.10.1. If its working for you... can you help me out for what I might be doing wrong... help would be really appreciated as this feature would really help us in production.

nik32 avatar Jul 03 '23 07:07 nik32

Working with this feature I've found a couple things that can confound testing...

  1. Your browser might matter and how you are going offline might matter - if you are using chrome network tab, there are gotchas with dev tools and workbox. You can get close.
  2. If you are using next-pwa dev/production matters. Also, the cached/installed version of the app matters. Try a production build then run start and open an incognito window. This helped narrow a lot of issues down for me.

With those solved:

<SessionProvider
  refetchInterval={60}
  refetchOnWindowFocus={typeof navigator !== "undefined" && navigator.onLine}
  refetchWhenOffline={false}
>

using typeof navigator !== "undefined" && navigator.onLine was flaky for me (especially in dev). The initial value was getting cached (HMR). If you change to

<SessionProvider
  refetchInterval={60}
  refetchOnWindowFocus={false}
  refetchWhenOffline={false}
>

It might reveal what was happening for you (at the cost of some functionality)

Also, if your page/component has useSession in it... that might be triggering a refresh. Try commenting that out and see if it helps narrow the issue. With these changes the session was working correctly for me.

jeffrafter avatar Aug 17 '23 21:08 jeffrafter