Stream multipart/related bodies

[simov]

👋

Related to this one #1181

My test case

request.post('https://www.googleapis.com/upload/drive/v2/files', {
  qs:{
    access_token:'...',
    uploadType:'multipart'
  },
  multipart: [
    {
      'Content-Type':'application/json',
      body: JSON.stringify({
        title:'cat.png'
      })
    },
    {
      'Content-Type':'image/png',
      body: fs.createReadStream('cat.png')
    }
  ]
},
function (err, res, body) {});

Basically I'm using the same module that form-data uses internally - https://github.com/felixge/node-combined-stream

Google implements their multipart/related support in much the same way, using multipart-stream which in turn uses sandwich-stream

[FredKSchott]

+1 @simov this is great. Can you add some tests to make sure this doesn't break in the future?

[simov]

Done. I just copied the form-data test, because the they are very similar. Also added a small fix to not modify the application level object

[FredKSchott]

since we already test that request behaves like a read stream elsewhere, this seems unnecessary. Since it now adds a dependency on the nodejs website (if the websites down, our tests don't pass) I'd say we should remove this remote_file test.

[FredKSchott]

If you copied this from test-form, can you remove it there as well?

[simov]

remote_file is used in the form-data test to show that the form-data module sets the name of the file properly inside the multipart body, which is required in some cases when using multipart/form-data uploads

As for the multipart/related test it doesn't really matter, as long as it's a stream it should work

Can we make the test server itself serves some image, and make request to it inside the test

[FredKSchott]

Awesome, +1 from me! (/w a small comment unrelated to the work itself)

[simov]

Done. Btw if the server doesn't return content-length header, the form-data module throws unhandled exception here https://github.com/felixge/node-form-data/blob/master/lib/form_data.js#L124 no idea if that's possible in a real use case scenario

[simov]

Btw what happened with this one? Is it good to go, or something is missing?

[nylen]

We have a 👍 from @FredKSchott, but there's a merge conflict now. If you can resolve the conflict then one of us will merge it.

[simov]

Fixed

[FredKSchott]

Awesome, merging now
+10 for losing the external dependency on nodejs.org here as well :)