apigee-127 / swagger-node-runner

The heart of Swagger-Node
MIT License
102 stars 123 forks source link

Hapi Plugin Decoration not present in Controller #99

Open arosboro opened 7 years ago

arosboro commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 6 years ago

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