node-fetch/fetch-blob

Doesn't work either with latest stable and beta version of node-fetch

octet-stream opened this issue ยท 6 comments

@jimmywarting As I mentioned here, I'm running into the problem when using this package with node-fetch. The problem remains both in the latest stable and beta. With fetch-blob@3.0.0-rc.0 and fetch-blob@2.1.1.

I was trying to test an example for my form-data-encoder where the encoder targeting Blob as you did in formdata-polyfill:

import {Readable} from "stream"

import {FormData, fileFromPath} from "formdata-node"
import {Encoder} from "form-data-encoder"

import Blob from "fetch-blob"
import fetch from "node-fetch"

async function toBlob(form) {
  const encoder = new Encoder(form)
  const chunks = []

  for await (const chunk of encoder) {
    chunks.push(chunk)
  }

  return new Blob(chunks, {type: encoder.contentType})
}

const fd = new FormData()

fd.set("name", "John Doe")
fd.set("avatar", await fileFromPath("avatar.png"))

const options = {
  method: "post",
  body: await toBlob(fd)
}

const response = await fetch("https://httpbin.org/post", options)

console.log(await response.text())

When I run this code node test.mjs this happens:

  1. With fetch-blob I get the following error: body.stream().pipe(dest);. This error appears in node-fetch/src/body.js:374:17
  2. When I run the same code, but with fetch-blob@2.1.1 it will lowercase the value of Blob#type which breaks boundary string returned by form-data-encoder.

Historically blob have always lowercased the type both in the constructor and in the blob.slice() method (so that is what we have also always done in fetch-blob - up until now).

(i know buffer.Blob in node v15.7 and up also lowercase the blob.type - i have already warn them about this and said it might not be such a good idea to do it and fetch-blob was thinking about removing this casting to lowercase)

Like you i also had this exact same issue 3years ago that i fixed by making the boundary only lowercased jimmywarting/FormData#17

looking at your code you also use uppercase characters
My recommendation to you is that you follow the same practise as FF and i did, by lowercasing the generated boundary so it dosen't have any effect when blob lowercase the type

- `FormDataBoundary${randomBytes(16).toString("hex")}`
+ `formdata-boundary${randomBytes(16).toString("hex")}`

If the users have a option to define there own boundary in the encoder constructor then i think that you should lowercase the boundary before it starts encoding

  constructor(form: FormDataLike, boundary: string = createBoundary()) {
    if (!isFormData(form)) {
      throw new TypeError("Expected first argument to be a FormData instance.")
    }

    if (typeof boundary !== "string") {
      throw new TypeError("Expected boundary to be a string.")
    }

-    this.boundary = boundary
+    this.boundary = boundary.toLowerCase()

cuz they might use buffer.Blob or some other that also lowercase the type

I reported this lowercasing issue at whatwg/html#6251 in late 2020 and they seems to agree that lowercasing the blob's type isn't that good as you loose the parameters "real" values in the blob.type officially (i think) they haven't changed the spec around that part yet. But what i think they are planing on doing is to make the hole type lowercased appart from the values... Meaning mimetype & keys are lowercase but the value isn't. (mime/type; key=VALUE)

I stripped this lowercase casting out of fetch-blob@3.0.0-rc.0 beta so it dose less harm and loosen it up a bit so it would be weird if you had this issue with the beta


could you try if this works code works with fetch-blob@3.0.0-rc.0 ?

import {FormData} from 'formdata-node'
import {Encoder} from 'form-data-encoder'
import Blob from "fetch-blob"

const encoder = new Encoder(new FormData())
const type = encoder.contentType
console.assert(new Blob([], { type }).type === type, "type stays the same")
console.assert(new Blob([], { type: 'foo/bar; key=VALUE' }).type === 'foo/bar; key=VALUE', "type stays the same")

form-data-encoder allows you to set your own boundary string as the second parameter in constructor. Tried this with nanoid, in that case the request is being send, but result remains the same. After your comment I remembered that their default alphabet has both capitalized and lowercased symbols. So, the problem with fetch-blob@2 can definitely be solved the way you described, thanks ๐Ÿ‘
Still, with fetch-blob doesn't seem to work with async iterables. But as far as I can tell, you're already working on it. That said, my problem is solved. Thank you again!

If the users have a option to define there own boundary in the encoder constructor then i think that you should lowercase the boundary before it starts encoding

Maybe I will do it to fix possible issues, or maybe I'll just recommend to do so if anybody runs into the same problem with this example.

I stripped this lowercase casting out of fetch-blob@3.0.0-rc.0 beta so it dose less harm and loosen it up a bit so it would be weird if you had this issue with the beta

Tested this with v3 โ€“ the type property stays the same as the original Encoder#contentType, so no such issue with v3

Still, with fetch-blob doesn't seem to work with async iterables. But as far as I can tell, you're already working on it. That said, my problem is solved. Thank you again!

blob#stream is iterable. Like i mention earlier, you may have a problem with checking if it's a buffer instead of uint8array

No, it's just me misspelled fetch-blob with node-fetch. I meant that node-fetch does not support Symbol.asyncIterator as request body yet. And as you can see in the top of this thread, node-fetch is trying to call .pipe() method on body.stream() which does not exists in async iterator in the first place.