Skip to content
This repository has been archived by the owner on Apr 23, 2019. It is now read-only.

Cannot read property 'on' of undefined - feathers-authentication-client #12

Closed
MHerszak opened this issue Dec 10, 2016 · 6 comments
Closed

Comments

@MHerszak
Copy link

Hi there,

I am having an issue with a package called feathers-authentication-client.

I am getting the following error:

Cannot read property 'on' of undefined

which points to feathers-authentication-client/lib/passport.js:80

and results in -> socket.io.engine.on('upgrade', function () {.

I can only guess, engine seems to be undefined?

Module versions
"feathers-authentication-client": "^0.1.3",

NodeJS version:
v6.9.1

Operating System:
macOs Sierra -v 10.12.1

@ekryski
Copy link
Member

ekryski commented Dec 12, 2016

I think the issue is that there is a race condition happening. It looks like engine isn’t assigned to the socket until a connection is established https://github.com/socketio/socket.io-client/blob/13002fb444e587ccdf2aa84711c8f0890d47b5ad/lib/manager.js#L208-L214.

So the event listener is registered before a connection has been established and therefore socket.engine may or may not be available yet. We need to move that handler to after the connection has occurred.

@ekryski
Copy link
Member

ekryski commented Dec 12, 2016

@MHerszak mentioned that he changed autoConnect: false to autoConnect: true and it worked. Which supports my thinking. Will work on a fix this week.

@bertho-zero
Copy link
Contributor

I had the same problem because I had to open the socket later, I had to make my app.configure('authentication') afterwards.

We could have something like this here (https://github.com/feathersjs/feathers-authentication-client/blob/master/src/passport.js#L55L77):

function socketUpgradeHandle() {
  socket.io.engine.on('upgrade', () => {
    debug('Socket upgrading');

    // If socket was already authenticated then re-authenticate
    // it with the server automatically.
    if (socket.authenticated) {
      const data = {
        strategy: this.options.jwtStrategy,
        accessToken: app.get('accessToken')
      };

      this.authenticateSocket(data, socket, emit)
        .then(this.setJWT)
        .catch(error => {
          debug('Error re-authenticating after socket upgrade', error);
          socket.authenticated = false;
          app.emit('reauthentication-error', error);
        });
    }
  });
}

if (socket.io) {
  if (socket.connected) {
    socketUpgradeHandle();
  } else {
    socket.on('connect', socketUpgradeHandle);
  }
}

@mihailp
Copy link

mihailp commented Jan 4, 2017

I have autoConnect: false and I face the same issue. Should this be fixed, or I need to solve it with some workaround like the guy above?

@marshallswain
Copy link
Member

@mihailp This one's still open. The code here needs to be moved https://github.com/feathersjs/feathers-authentication-client/blob/master/src/passport.js#L56. I'm just not sure where, yet.

@daffl
Copy link
Member

daffl commented Jan 29, 2017

Should be closed via #20

@daffl daffl closed this as completed Jan 29, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants