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

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

Closed
mikicho opened this issue Sep 29, 2023 · 4 comments · Fixed by #515
Closed

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

mikicho opened this issue Sep 29, 2023 · 4 comments · Fixed by #515

Comments

@mikicho
Copy link
Contributor

mikicho commented Sep 29, 2023

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
Copy link
Member

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
Copy link
Contributor Author

mikicho commented Dec 21, 2023

@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
Copy link
Contributor Author

mikicho commented Dec 21, 2023

I don't want to forget what I did here (or that I did lol), so I opened a PR: #493

Happy holidays :)

@kettanaito
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants