mswjs / interceptors

Low-level network interception library.
https://npm.im/@mswjs/interceptors
MIT License
535 stars 120 forks source link

req.write should throw error where chunk is undefined #458

Closed mikicho closed 1 month ago

mikicho commented 11 months ago

https://github.com/mswjs/interceptors/blob/main/src/interceptors/ClientRequest/NodeClientRequest.ts#L101

chunk must not be undefined and throw:

TypeError [ERR_INVALID_ARG_TYPE]: The "chunk" argument must be of type string or an instance of Buffer or Uint8Array. Received undefined

I can open a PR, but because you ignored this case positively, I want to ensure I don't miss something.

About the solution, we can either:

  1. call to super.write() and let Node emit the error
  2. Return a custom error
kettanaito commented 10 months ago

Hi, @mikicho. This is a good suggestion.

Counting on super.write() won't work because I'm sure Node will throw something connection-related if the socket connection failed (think of the mocked requests to non-existing hosts) and you try to write onto that socket.

I hate to re-create internal Node errors so let's brainstorm how we can achieve this behavior without repeating Node.

mikicho commented 8 months ago

@kettanaito It seems like the input validation is the first thing Node does before trying to write into the socket (source) so we can do something like:

class NodeClientRequest {
  ...
  write(...args) {
     ...
    const [chunk, encoding, callback] = normalizeClientRequestWriteArgs(args)
    if (!chunk) {
      super.write()
    }
  }
}

This test passes:

it('does not allow empty chunk', async () => {
  const emitter = new Emitter<HttpRequestEventMap>()
  const request = new NodeClientRequest(
    normalizeClientRequestArgs('http:', httpServer.http.url('/comment'), {
      method: 'POST',
    }),
    {
      emitter,
      logger,
    }
  )

  // @ts-expect-error - test undefined chunk
  expect(() => request.write()).toThrow('The "chunk" argument must be of type string or an instance of Buffer or Uint8Array. Received undefined')
  // @ts-expect-error - test null chunk
  expect(() => request.write(null)).toThrow('May not write null values to stream')
})
mikicho commented 8 months ago

I don't want to forget what I did here (or that I did lol), so I opened a PR: https://github.com/mswjs/interceptors/pull/493

Happy holidays :)

kettanaito commented 1 month ago

Released: v0.32.0 🎉

This has been released in v0.32.0!

Make sure to always update to the latest version (npm i @mswjs/interceptors@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.