martyjs / marty

A Javascript library for state management in React applications
http://martyjs.org
MIT License
1.09k stars 77 forks source link

Server side fetches do not calculate content-length header #364

Closed duro closed 8 years ago

duro commented 8 years ago

I have found an odd issue. It seems that when fetches are handed on the server they do not calculate the content-length header value. However, when I have a fetch that triggers on the client/browser, and then hits the server, that header is present.

My UI app is interfacing with another internal API that lives in a different app built on top of Ruby on Rails. This API requires that the content-length header be present. So calls from the client/browser using fetch direct to this API work fine, bit when I try to use an HttpStateSource hook to send these same request from our node server to the Rails server, that header is not calculated.

I have worked around this by calculating the value myself and adding it to the headers of the request before it goes out. This is easy for stringifyable things like JSON, but I worry that not having this handled intelligently by the fetch library could present a problem down the road when we start sending things like multi-part data to the API.

Is this a bug or a "feature"? Am I doing something wrong?

taion commented 8 years ago

fetch goes through a different polyfill on server v on browser - node-fetch on server v whatwg-fetch on browser (both via isomorphic-fetch). You could look at those.

taion commented 8 years ago

I'm actually not seeing what you report. If I run:

import {HttpStateSource} from 'marty';

HttpStateSource.removeHook('parseJSON');
const source = new HttpStateSource({});

source.post({
  url: 'http://httpbin.org/post',
  body: 'a=1'
})
  .then(res => res.json())
  .then(json => {
    console.log('state source');
    console.log(json)
  });

fetch(
  'http://httpbin.org/post',
  {body: 'a=1', method: 'POST'}
)
  .then(res => res.json())
  .then(json => {
    console.log('node-fetch');
    console.log(json)
  });

I get:

state source
{ args: {},
  data: 'a=1',
  files: {},
  form: {},
  headers:
   { Accept: '*/*',
     'Accept-Encoding': 'gzip,deflate',
     'Content-Length': '3',
     Host: 'httpbin.org',
     'User-Agent': 'node-fetch/1.0 (+https://github.com/bitinn/node-fetch)' },
  json: null,
  origin: '104.226.1.2',
  url: 'http://httpbin.org/post' }
node-fetch
{ args: {},
  data: 'a=1',
  files: {},
  form: {},
  headers:
   { Accept: '*/*',
     'Accept-Encoding': 'gzip,deflate',
     'Content-Length': '3',
     Host: 'httpbin.org',
     'User-Agent': 'node-fetch/1.0 (+https://github.com/bitinn/node-fetch)' },
  json: null,
  origin: '104.226.1.2',
  url: 'http://httpbin.org/post' }

i.e. the same behavior, with Content-Length properly supplied in both cases.

Let me know if you see further issues.

duro commented 8 years ago

And you tested this running the system through app.renderToString?

Becuase when I have the ran through renderToString and then insert a before hook. The request that is passed to the before hook does not have a content-length header.

Also, what you are sending to console log is the response headers. My issue is that the request headers do not have a content-length.

And the API I am hitting requires that on the request.

taion commented 8 years ago

The httpbin.org/post endpoint echoes back the request headers - note the "Accept" settings and "User-Agent".

Your "before" hook will not see the content-length header, because the content-length header is generated after that, i.e. the before hooks are all run before we call into fetch.

duro commented 8 years ago

Gotcha. However, when I have fetch send route the request right back into my node API, and then I look at the request coming into that endpoint, I don't have a content-length header at the endpoint. Right now I have hacked around this by adding the header in the before hook, and this resolves the issue. That said, this feels like a hack, and something that will trip me up down the line.

taion commented 8 years ago

Try to see if you can reproduce the issue with node-fetch (via isomorphic-fetch) on its own. What you're describing sounds a lot like an issue there, which you should file as an issue w/the node-fetch devs if possible.