bnoguchi / everyauth

node.js auth package (password, facebook, & more) for Connect and Express apps
http://everyauth.com/
3.49k stars 447 forks source link

Null dereference on twitter error #111

Open ghost opened 12 years ago

ghost commented 12 years ago

Occasionally twitter login fails with a null dereference, crashing the app. Here is the stack crawl:

/path/to/mynodeapp/node_modules/everyauth/lib/modules/twitter.js:35
    return new Error(data.data.match(/<error>(.+)<\/error>/)[1]);
                                                            ^
TypeError: Cannot read property '1' of null
    at /path/to/mynodeapp/node_modules/everyauth/lib/modules/twitter.js:35:61
    at /path/to/mynodeapp/node_modules/everyauth/lib/step.js:97:21
    at [object Object].fail (/path/to/mynodeapp/node_modules/everyauth/lib/promise.js:53:24)
    at /path/to/mynodeapp/node_modules/everyauth/lib/modules/oauth.js:168:24
    at /path/to/mynodeapp/node_modules/everyauth/node_modules/oauth/lib/oauth.js:432:22
    at passBackControl (/path/to/mynodeapp/node_modules/everyauth/node_modules/oauth/lib/oauth.js:366:13)
    at IncomingMessage.<anonymous> (/path/to/mynodeapp/node_modules/everyauth/node_modules/oauth/lib/oauth.js:378:9)
    at IncomingMessage.emit (events.js:81:20)
    at HTTPParser.onMessageComplete (http.js:133:23)
    at CleartextStream.ondata (http.js:1231:22)
bnoguchi commented 12 years ago

@coalitions Just before the return on line 35 of lib/modules/twitter.js, could you add a console.log(data); and console.trace(), and then tell me what the output s on the occasional failure? i.e.

  console.log(data);
  console.trace();
  return new Error(data.data.match(/(.+)<\/error>/)[1]);
ghost commented 12 years ago

Thanks Brian, I've made that change. But after about 50 attempts to trigger the intermittent problem I haven't succeeded. It's possible that this is timeout related, and more likely to happen when twitter is under higher load than it is now. Note that I even tried lowering the moduleTimeout for twitter to 5ms, which causes an error to be logged on every login request, yet the login succeeds. That seems odd. This is what I see:

GET /auth/twitter 133
...finished step
starting step - extractTokenAndVerifier
...finished step
starting step - getSession
...finished step
starting step - rememberTokenSecret
...finished step
starting step - getAccessToken
Everyauth error: { stack: [Getter/Setter],
arguments: undefined,
type: undefined,
message: 'Step getAccessToken of twitter module timed out.' }
...finished step
starting step - fetchOAuthUser
Everyauth error: { stack: [Getter/Setter],
arguments: undefined,
type: undefined,
message: 'Step fetchOAuthUser of twitter module timed out.' }
...finished step
starting step - findOrCreateUser
...finished step
starting step - compileAuth
...finished step
starting step - addToSession
...finished step
starting step - sendResponse

Does this make sense to you?

vinkaga commented 12 years ago

I can reproduce this issue by running node.js in Ubuntu guest in VMWare player (windows host) and making the host sleep and then wake up. After waking up, the twitter auth shows this error. If I shutdown and restart Ubuntu guest, the error goes away.

Here is the output after putting log & trace statements as you suggested.

{ statusCode: 401,
  data: 'Failed to validate oauth signature and token' }
Trace: 
    at /home/vinay/Projects/nodeapp/node_modules/everyauth/lib/modules/twitter.js:36:11
    at /home/vinay/Projects/nodeapp/node_modules/everyauth/lib/step.js:97:21
    at [object Object].fail (/home/vinay/Projects/nodeapp/node_modules/everyauth/lib/promise.js:53:24)
    at /home/vinay/Projects/nodeapp/node_modules/everyauth/lib/modules/oauth.js:105:18
    at /home/vinay/Projects/nodeapp/node_modules/everyauth/node_modules/oauth/lib/oauth.js:510:17
    at passBackControl (/home/vinay/Projects/nodeapp/node_modules/everyauth/node_modules/oauth/lib/oauth.js:366:13)
    at IncomingMessage.<anonymous> (/home/vinay/Projects/nodeapp/node_modules/everyauth/node_modules/oauth/lib/oauth.js:378:9)
    at IncomingMessage.emit (events.js:88:20)
    at HTTPParser.onMessageComplete (http.js:137:23)
    at CleartextStream.ondata (http.js:1124:24)
soggie commented 12 years ago

We are also facing this issue in our production server too, which runs Ubuntu too. We can also reproduce this on Mint. I'll see if we can find out a fix or interim solution for now, since I suspect this might be a deeper issue.

ghost commented 12 years ago

Any updates? I have the same issue with VirtualBox running CentOS.

dylang commented 12 years ago

I get this issue too.

/home/node/local/lib/node_modules/everyauth/lib/modules/twitter.js:35
    return new Error(data.data.match(/<error>(.+)<\/error>/)[1]);
                                                            ^
TypeError: Cannot read property '1' of null
    at /home/node/local/lib/node_modules/everyauth/lib/modules/twitter.js:35:61
    at /home/node/local/lib/node_modules/everyauth/lib/step.js:97:21
    at [object Object].fail (/home/node/local/lib/node_modules/everyauth/lib/promise.js:53:24)
    at /home/node/local/lib/node_modules/everyauth/lib/modules/twitter.js:14:31
    at passBackControl (/home/node/local/lib/node_modules/everyauth/node_modules/oauth/lib/oauth.js:366:13)
    at IncomingMessage.<anonymous> (/home/node/local/lib/node_modules/everyauth/node_modules/oauth/lib/oauth.js:378:9)
    at IncomingMessage.emit (events.js:88:20)
    at HTTPParser.onMessageComplete (http.js:137:23)
    at CleartextStream.ondata (http.js:1137:24)
    at CleartextStream._push (tls.js:367:27)

Can you change it to only look for [1] if the match is successful?

fulmicoton commented 12 years ago

I got a similar issue because I had a problem with my app consumeKey. data.data was simply the text "Failed to validate oauth signature and token".

I also think this [1] without checking that there was a match is very dangerous. Especially in error handling code.

dylang commented 11 years ago

We still have this problem, ranges from zero to 100 times a day on http://DoodleOrDie.com. It's the only reason our Node service crashes and it's driving us nuts.

labe-me commented 11 years ago

Got the JSON parse error, hacked it with :

        if (data.indexOf("for maintenance") != -1)
            promise.fail("TWITTER DOWN FOR MAINTENANCE");