senecajs / seneca-web

Http route mapping for Seneca microservices.
MIT License
76 stars 44 forks source link

Can't use BodyParser and PUT method #105

Closed jsmadja closed 8 years ago

jsmadja commented 8 years ago

Hi,

I've got a problem. I was using Seneca 2.1 (before seneca-web became an external dependency with Seneca 3.x).

I'm trying to migrate my code and I have a problem with PUT request. They were in timeout. I've found the problem and it "works" when I remove

express.use(BodyParser.urlencoded({extended: true})) .use(BodyParser.json({limit: '50mb'}));

What I don't understand is : why I can't have a body auto-parsed like I had in Seneca 2.1

Without the BodyParser, I can see a msg.args.body as a string and not as an object.

Here the minimal code to reproduce my problem. Or you can git clone my repo : https://github.com/jsmadja/seneca-web-put

index.js

const Seneca = require('seneca');
const SenecaWeb = require('seneca-web');
const Express = require('express');
const seneca = Seneca();
const BodyParser = require('body-parser');
const Routes = require('./routes');
const Plugin = require('./plugin');
const express = Express()
    .use(BodyParser.urlencoded({extended: true}))
    .use(BodyParser.json({limit: '50mb'}));

seneca
    .use(SenecaWeb, {
        routes: Routes,
        context: express,
        adapter: require('seneca-web-adapter-express'),
        parseBody: false
    })
    .use(Plugin)
    .ready(() => seneca.export('web/context')().listen(3000));

routes.js

'use strict';

module.exports = [
{
    prefix: '/todo',
    pin: 'role:todo,cmd:*',
    map: {
        list: {
            GET: true, PUT: true
        }
    }
}
];

plugin.js

'use strict';

module.exports = function plugin() {
    var seneca = this;
    seneca.add('role:todo,cmd:list', (msg, done) => {
        done(null, {ok: true});
    });
};
tswaters commented 8 years ago

Take a look at #92 - you can disable the body parser by providing the relevant option. I didn't try it with PUT but it should work the same way

jsmadja commented 8 years ago

As you can see, I'm using the parameter parseBody: false. I tried with true value, but my problem is I don't know why my body is not auto-unmarshalled (string to object) anymore since my migration from Seneca 2.1.

mcdonnelldean commented 8 years ago

@jsmadja The current behaviour with parseBody:true should work for you, if not, it is a bug. #92 added the ability to turn it off, not remove it.

What output are you getting with parseBody defaulted to true? We may need to make parseBody:true more robust. The old implementation used Express' body parser to auto marshal. This became a problem when we went to implement things like hapi.

The new body parser is pretty lightweight and more to enable basic fallback. We can certainly enhance the new parser but I would be against adding back in BodyParser as it is something that is best configured external to seneca-web.

As for why the change generally. Seneca web became super hard to maintain and was becoming stagnant. Features took far too long to add and load order was unreliable. Overall the minor configuration shortcuts were not worth it in the end.

tswaters commented 8 years ago

Apologies, I didn't read your issue as close as I should have. After looking a little closer I think I see the problem: the option needs to be passed like the following -- wrapped in an options key -- see readme:

    .use(SenecaWeb, {
        routes: Routes,
        context: express,
        adapter: require('seneca-web-adapter-express'),
        options: {parseBody: false}
    })

The behavior you are seeing (stringified response) is the way the built-in body parser works currently. With both built-in parser and body-parser you'll see requests going off into the ether.

jsmadja commented 8 years ago

It works. I made a pull request with more documentation.