electric icon indicating copy to clipboard operation
electric copied to clipboard

Best pattern for handling 401/403 token expiry?

Open thruflo opened this issue 1 year ago • 6 comments

I've written a little client for the new gatekeeper auth pattern as per https://github.com/electric-sql/electric/pull/1963

Basically I need to handle token expiry and when a token expires, get a new one and then reconnect. I'm doing this with the Shape and ShapeStream: https://github.com/electric-sql/electric/blob/thruflo/auth-guide/examples/gatekeeper-auth/client/index.ts

I have this working, but AFAICS it requires having access to the stream instance to process messages to manually keep track of the last offset. I wonder:

  • could the ShapeStream provide public read access to lastOffset
  • what about code that's using useShape or doesn't want to be handling a shape stream?

I'm thinking that anywhere we support providing auth headers, we should have an easy way of handling a FetchError and resetting them to reconnect with new auth (or whatever).

What do you think?

thruflo avatar Nov 15 '24 17:11 thruflo

Getting a fresh token had come up before and one idea is that for headers/params could take an async function vs. a straight value — then that function would ensure that the token is still valid on each request.

KyleAMathews avatar Nov 15 '24 18:11 KyleAMathews

Yup, that’s good.

There’s also the control flow issue that atm, the ShapeStream just stops on a 4xx status. So I believe a Shape or a useShape with a headers option just isn’t going to try to reconnect.

It might be nice to be able to explicitly configure a function that handles 4xx errors where the return value determines whether to retry with new request config or not?

Or is this what the fetchClient option is already able to support?

thruflo avatar Nov 15 '24 19:11 thruflo

yeah there's no way to automatically restart a shapestream if it errors. We have a problem that in general there's no way to easily listen for errors (and optionally do something about them).

KyleAMathews avatar Nov 15 '24 19:11 KyleAMathews

If we want to "restart" by creating a new stream, then this will help: https://github.com/electric-sql/electric/pull/1989

thruflo avatar Nov 18 '24 18:11 thruflo

@balegas @kevin-dp @msfstef and I chatted about this a bit earlier (amongst other topics). We were discussing adding a global onError handler to streams for https://github.com/electric-sql/electric/pull/1985 — as part of that, people could handle certain types of errors by returning an update to some of the stream options. Then internally the stream could retry with the new options.

So e.g.

new ShapeStream({
  url,
  params: {
    token,
  },
  onError: async (error) => {
    if (error instanceof FetchError & error.status === 401) {
      const token = await getToken()
      return { params: { token }}
    }
  },
})

People should only be able to update probably headers and query params — basically things that don't change the essential nature of the stream. Most errors aren't recoverable from so we shouldn't let people try.

KyleAMathews avatar Nov 19 '24 15:11 KyleAMathews

I think this can work nicely. Avoids having to manually restart or do the “auto-retry calling headers function again” approach — this is more explicit, less magic.

Being in the ShapeStreamOptions means it will naturally be threaded through from useShape.

thruflo avatar Nov 19 '24 15:11 thruflo

We now provide the onError handler and have docs for this use case, and we have https://github.com/electric-sql/electric/pull/2184 as well to improve on it.

Should we close this issue?

msfstef avatar Dec 19 '24 11:12 msfstef