oneblink / hapi-oauth2orize

A bridge between hapi and OAuth2orize
BSD 3-Clause "New" or "Revised" License
27 stars 17 forks source link

authorize endpoint: Uncaught error: reply interface called twice #14

Open pabshazon opened 8 years ago

pabshazon commented 8 years ago

Hello,

This is my code for controllers/oauth2.js where I have the handler for '/oauth/authorize'

   authorize: function (request, reply) {
      oauth.authorize(request, reply, function (req, res) {
          reply.view('oauth', {transactionID: req.app.transactionID});
      }, function (clientID, redirect, done) {
        server.helpers.find('client', clientID, function (docs) {
          done(null, docs[0], docs[0].redirect_uri);
        });
      });
    },

In the third line I get this error stack trace:

Debug: internal, implementation, error 
   ** Error: Uncaught error: reply interface called twice**
    at Object.exports.contain.exports.reachTemplate.exports.assert.condition [as assert] (/home/pablo/devLab/audienceLeap/api/server/src/node_modules/hapi/node_modules/hoek/lib/index.js:732:11)
    at Function.internals.Reply.interface.internals.response (/home/pablo/devLab/audienceLeap/api/server/src/node_modules/hapi/lib/reply.js:132:10)
    at reply (/home/pablo/devLab/audienceLeap/api/server/src/node_modules/hapi/lib/reply.js:70:22)
    at Object.ExpressServer.res.end (/home/pablo/devLab/audienceLeap/api/server/src/node_modules/hapi-oauth2orize/index.js:271:32)
    at errorHandler (/home/pablo/devLab/audienceLeap/api/server/src/node_modules/oauth2orize/lib/middleware/errorHandler.js:67:18)
    at /home/pablo/devLab/audienceLeap/api/server/src/node_modules/hapi-oauth2orize/index.js:91:59
    at errorHandler (/home/pablo/devLab/audienceLeap/api/server/src/node_modules/oauth2orize/lib/middleware/errorHandler.js:73:60)
    at /home/pablo/devLab/audienceLeap/api/server/src/node_modules/hapi-oauth2orize/index.js:88:57
    at /home/pablo/devLab/audienceLeap/api/server/src/node_modules/oauth2orize/lib/middleware/authorization.js:121:25
    at pass (/home/pablo/devLab/audienceLeap/api/server/src/node_modules/oauth2orize/lib/server.js:283:14)
    at Server._parse (/home/pablo/devLab/audienceLeap/api/server/src/node_modules/oauth2orize/lib/server.js:285:5)
    at authorization (/home/pablo/devLab/audienceLeap/api/server/src/node_modules/oauth2orize/lib/middleware/authorization.js:120:12)
    at Object.internals.authorize (/home/pablo/devLab/audienceLeap/api/server/src/node_modules/hapi-oauth2orize/index.js:85:66)
    at oauth2.authorization (/home/pablo/devLab/audienceLeap/api/server/src/controllers/oauth2.js:103:13)
    at Object.exports.execute.internals.prerequisites.internals.handler.callback [as handler] (/home/pablo/devLab/audienceLeap/api/server/src/node_modules/hapi/lib/handler.js:96:36)
    at /home/pablo/devLab/audienceLeap/api/server/src/node_modules/hapi/lib/handler.js:30:23

Any ideas how we fix this?

devinivy commented 8 years ago

Looks like it's due to some inconsistent error-handling around here: https://github.com/blinkmobile/hapi-oauth2orize/blob/master/index.js#L81-L98 . Your code generated an error which propagated down into a duplicate call to reply(). This area of the code clearly needs a little bit of love :)

pabshazon commented 8 years ago

Yes, the error inconsistently handled is that I was missing ?response_type= in the call.

Now I'm getting the same "reply called twice" because of a simple error of "cannot read property find of undefined", I suppose I need to create my own models now for clients, tokens, etc... instead of using the example code :)

Do you have any suggestion how to fix this error? Any related documentation or approach? Usually I understand I cannot reply twice inside a function so I separate the two reply calls by and if else, but here the two reply calls are being made from different functions (inside and callback of oauth.authorize()).

My approach would be modifying the callback of authorize(request, reply, callback(req, res)) so it passes a variable called error with req and es and then we can if/else the two different replies with error or view.

Comments welcome!

pabshazon commented 8 years ago

I have improved the code a little bit with this in my code (modifying the example code from README and commenting a line in index.js (169))

Comment this line, so we can see the actual error message: https://github.com/blinkmobile/hapi-oauth2orize/blob/master/index.js#L169

Modify the oauth.authorize() callback like this, adding if(!request.response.isBoom)

    authorize: function (request, reply) {
      oauth.authorize(request, reply, function (req, res) {
        if(!request.response.isBoom) {
          reply.view('oauth', {transactionID: req.app.transactionID});
        }
      }, function (clientID, redirect, done) {
        server.helpers.find('client', clientID, function (docs) {
          done(null, docs[0], docs[0].redirect_uri);
        });
      });
    },

So far working for me... but I'm showing some internal errors in the API output haha. Maybe you can just not comment the line 169 so we "hide" the error from api users, but we still need a way to throw the error with the stack trace in console then.