Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hapi Plugin Decoration not present in Controller #99

Open
arosboro opened this issue Apr 14, 2017 · 9 comments
Open

Hapi Plugin Decoration not present in Controller #99

arosboro opened this issue Apr 14, 2017 · 9 comments

Comments

@arosboro
Copy link

arosboro commented Apr 14, 2017

I'm attempting to use Chairo with happi to make a rest api which interfaces with seneca services.

I'm including both plugins with register:

    app.register([{ register: Chairo, options: options.seneca }, swaggerHapi.plugin], function (err) {
        if (err) { return console.error('Failed to load plugin:', err); }

        app.seneca.client(options.baseTransport)

        app.start(function() {
          if (swaggerHapi.runner.swagger.paths['/hello']) {
            console.log('try this:\ncurl http://127.0.0.1:' + port + '/hello?name=Scott');
          }
        });
      });

Chairo decorates app, and req with an instance of seneca. However I'm not sure bagpipes is using the hapi request with my controller. req.seneca is undefined when I make a request to my controller. Not being familiar with hapi, shouldn't all registered plugins be available for use together?

@arosboro
Copy link
Author

arosboro commented Apr 16, 2017

I found the issue is related to the raw nodejs request being used:

lib/hapi_middleware.js:28

      server.ext('onRequest', function(request, reply) {

        var req = request.raw.req;
        req = request;
        var res = newResponse(reply);

@markhowells
Copy link

I wish I'd found this issue before I spent 3 hours tracking down the issue and arriving at the same solution. :)
I'm testing the above change with no apparent problems

@markhowells
Copy link

@arosboro I've discovered it's much more complex than this simple fix.... For example the bodyParser (for parameters) expect the req object to be the raw request. I'm working through the code now to see if there's a straightforward way a resolution can be achieved.

@arosboro
Copy link
Author

great, hopefully you can figure something out. I don't have many dependencies for other middleware, but I'm glad this is getting attention.

@markhowells
Copy link

@theganyo - is this something I should work on more? (I guess the question could be "am I using swagger-node-runner correctly"). The issue here is that the hapi_middleware replaces the 'internals' request object that hapi passes around. I'm guessing it does so in order that the bodyParser has access to teh input stream. The consequernce of this is that a controller can't use any other middleware that decorates the request object- effectively rendering this project useless for anything other than trivial apps. (We use Dogwater/Waterline ORM for example which expects you to have access to req.server and req.dogwater() etc).
I've hacked up a solution that works for me - but it involved reworking swagger_params_parser and has introduced Hapi specific changes into there....
I could probably intoduce some hapi specific shims into the default.yml to do a similar thing (although I don't know bagpipes at all).
However, I'm left wondering whether I've missed something somewhere because the changes are awkward and the omission of middleware in the controller so fundamental, that perhaps there's a mechanism I'm not using properly.
I'm happy to develop a solution if necessary, but some guidance on how to proceed would be appreciated. :)

@theganyo
Copy link
Collaborator

theganyo commented Jun 9, 2017

Hey Mark,
I'm afraid I'm no expert on Hapi. It has it's own way of doing things and it might take a bit to get the integration just right. For what it's worth, I'd definitely prefer a solution that can be isolated to the Hapi middleware rather than spread into all the other code. So, if the solution could be added to the hapi middleware or encoded into a fitting, that would be best.

@markhowells
Copy link

Developing a fitting is probably the best way forward - but it'll take me some time. Until then, I'm afraid the swagger-hapi module is pretty much useful only for trivial tasks. I'll see if I can migrate the deltas into a fitting - at least that'll isolate them from the parameter parsing codebase. Do you know if there was any other reason (other than parameter parsing that is) why the raw request was selected by the hapi middleware?

@theganyo
Copy link
Collaborator

theganyo commented Jun 9, 2017

Yes, this issue is quite unfortunate. No, I don't know exactly why the raw request was necessary... but I think that the Hapi request had a naming collision with some method or property on the node request that caused it to have different semantics. Sorry, I don't have more specific information on that.

@joshualim92
Copy link

Could we deep clone request.raw, pass it through the chain, then replace request.raw with the post-chained req?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants