feathers icon indicating copy to clipboard operation
feathers copied to clipboard

Feathers authentiation client handleSocket dosen't honour timeout

Open jeffborg opened this issue 4 years ago • 3 comments

it seems that the method handleSocket in AuthenticationClient linked below here which recreates authentication once socket.io or primus reconnects doesn't honor the timeout which is used when actually calling the service.

If socket.io doesn't reconnect, could be 404 on server or server offline or whatever then the promise is never resolved.

https://github.com/feathersjs/feathers/blob/crow/packages/authentication-client/src/core.ts#L66

What happens is that the service calls will never resolve or reject because we are stuck here waiting for socket io to reconnect. from the hook https://github.com/feathersjs/feathers/blob/crow/packages/authentication-client/src/hooks/authentication.ts#L12

I think this timeout should actually be implemented in the hook at least or the actual handleSocket or by wrapping get('authentication') which returns a new promise everytime and uses the real auth promise. It can then reject on timeout but the next consumer will get a new timeout.

I have worked around it in our frontend app by placing a before hook which tries to get the authentication promise and if it times out throwing an error.

    feathers.hooks({
      before: {
        all: [(context) => {
          const { app, path, method, service, app: { authentication: authService } } = context
          // bypass this for the actual auth service
          if (stripSlashes(authService.options.path) === path && method === 'create') {
            return context
          }
          return new Promise((resolve, reject) => {
            // create future timer for timeout MS in future
            const timeoutId = setTimeout(() => reject(
              new Timeout(`Timeout of ${service.timeout}ms exceeded getting auth to call ${method} on ${path}`, {
                timeout: service.timeout,
                method,
                path
              })
            ), service.timeout)
            Promise.resolve(app.get('authentication')).then(() => {
              clearTimeout(timeoutId)
              resolve(context)
            }).catch(reject)
          })
        }]
      }
    })

jeffborg avatar Dec 02 '20 00:12 jeffborg

@jeffborg I ran into a similar issue while switching to primus from socketIO. The reauth wasn't being called correctly if the server disconnected. In my case I keep track of logged in user through my frontend's store so I just overwrote the authentication promise to force it to verify both feathers authenticated prop or my store to see if it should try to reAuthenticate.

// Resets auth revalidation to force revalidate 
// if our store has a logged in user
const authPromise = new Promise(resolve =>
  feathers().primus.once('open', (data: any) => {
    resolve(data);
  })
)
// Only reconnect when `reAuthenticate()`,  `authenticate()`, or we have a user
// has been called explicitly first
// Force reauthentication with the server
.then(() => {
  return feathers().authenticated || store.getState().auth.userId ? feathers().reAuthenticate(true) : null
});
feathers().set('authentication', authPromise);

feathers() method there is just a wrapper around the feathers app object as we provide it through an abstracted react lib.

You could prob take a similar approach to get the auth promise to return like you'd like vs adding it all as a second hook.

jchamb avatar Jan 12 '21 15:01 jchamb

@jchamb our problem in testing was that lets say you network connection was flakley or went away then feathers would never reject your call. It does this for normal calls but not when recreating the authentiation before making the call. see the 2 sections of code referenced above. I believe if everything is normal feathers does reauth correctly with the socket io, so this may be a 2nd bug with the primus.

jeffborg avatar Jan 12 '21 20:01 jeffborg

What you have looks solid. I know our problems were not exact same thing. Was just pointing out you could overwrite the actual auth promise yourself to provide same sort of timeout mechanism at the south promise level since you mentioned potentially adding the timeout there in your description.

jchamb avatar Jan 12 '21 20:01 jchamb