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

Adding body-parser-midddleware breaks proxy #363

Closed
ashleahhill opened this issue Sep 21, 2016 · 8 comments
Closed

Adding body-parser-midddleware breaks proxy #363

ashleahhill opened this issue Sep 21, 2016 · 8 comments

Comments

@ashleahhill
Copy link
Contributor

ashleahhill commented Sep 21, 2016

Our application uses json-server as a module to fill in the holes in an existing API. As a result, we rely pretty heavily on proxy functionality provided with http-proxy-middleware.

Unfortunately, a recent update to json-server has broken application/json POST requests through our proxy because adding body-body parser as top-level middleware in #338 affects the request sent through the proxy.

After encountering the issue and evaluating how our application has grown and what it needs, I've realized that it probably no longer fits the intended use json-server very well. However, for the benefit of others that are using jsons-server as a module, you may want to bump the version number with this added feature.

@typicode
Copy link
Owner

Hi @ashleahhill,

Thanks a lot for letting me know about this. I'd rather fix it if possible.

I guess that there's some "interaction" with middlewares that you're using. I don't know if it's possible, but maybe you can try mounting JSON Server's middlewares on another path:

server.use('/api', middlewares)
server.use('/api', router)

If it doesn't work, how can I reproduce the issue with http-proxy-middleware? Do you have some simple code that fails with this version?

@ashleahhill
Copy link
Contributor Author

ashleahhill commented Sep 28, 2016

Hi, @typicode ,

Sorry for the delay.

Is there an error in your example? Did you mean something like this?

server.use('/api/json', middlewares)
server.use('/api/otherstuff', proxyMiddleware)
server.use('/api/json', router)

The setup for the proxy isn't suuuuper straightforward

  1. We create the express server with jsonServer.create()
  2. We add the jsonServer.defaults() middleware
  3. Config that is basically an array of proxy route is read
  4. We iterate over that array and add a proxy for every route in the config
  5. We add the jsonServer.router() middleware with our data.
  6. We start the server

Since we are already only putting the proxy middleware on paths that aren't served by JSON Server, I don't think that that solution will work.

I vaguely remember that express applies middleware in the order they are received, so I'll try adding the proxy config before the JSON Server middleware and report back.

@ashleahhill
Copy link
Contributor Author

Tried it. Body Parser still applies to every POST application/json request.

I could add a layer that checks every request and, if its not a path configured to be proxied to the real API, proxy it to the JSON Server instance. But I'm not sure if that's the approach I want to take.

@chimurai
Copy link

Hi @ashleahhill

Try restreaming the parsed body:
chimurai/http-proxy-middleware#40 (comment)

@ashleahhill
Copy link
Contributor Author

Thanks @chimurai,

I thought of that, but it felt hacky to me considering the cause.

At the moment, our application won't be able to use JSON server enough to justify it while #372 is out there.

@typicode
Copy link
Owner

typicode commented Oct 4, 2016

I would suggest sticking with the previous version. I may simply remove body-parser from jsonServer.defaults() as it seems to be a tricky issue.

And thank you for the detailed steps to reproduce.

@typicode
Copy link
Owner

Hi @ashleahhill,

I've published v0.9.0-beta.2 which should fix it.

If you want, you can give it a try using npm install json-server@next.
README for v0.9.0 can be found in the next branch https://github.com/typicode/json-server/tree/next

Feedback is welcome :)

@typicode
Copy link
Owner

typicode commented Nov 12, 2016

Closing it since v0.9.0 is now published :)

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

3 participants