Skip to content

Conversation

@airhorns
Copy link
Member

@airhorns airhorns commented Oct 5, 2020

Before this change, if a Fastify user attached an onTimeout hook to their server, an error would be thrown in test mode when injecting requests with light-my-request

TypeError: request.raw.socket.on is not a function

      at Object.routeHandler [as handler] (../../node_modules/fastify/lib/route.js:350:28)
      at Router.lookup (../../node_modules/find-my-way/index.js:356:14)
      at ../../node_modules/light-my-request/index.js:101:38
      at Request.prepare (../../node_modules/light-my-request/lib/request.js:110:12)
      at ../../node_modules/light-my-request/index.js:101:11
      at doInject (../../node_modules/light-my-request/index.js:97:12)
      at Chain.<computed> [as then] (../../node_modules/light-my-request/index.js:187:23)
          at runMicrotasks (<anonymous>)

This fixes the socket object to have an on function (as well as the rest of the EventEmitter interface) so that things that add timeout handlers can at least add them without erroring. This doesn't implement actual timeout injection, but I think that could be done in userland by emitting events on the socket manually.

Similar to #99.

Before this change, if a Fastify user attached an `onTimeout` hook to their server, an error would be thrown in test mode when injecting requests with light-my-request

```
TypeError: request.raw.socket.on is not a function

      at Object.routeHandler [as handler] (../../node_modules/fastify/lib/route.js:350:28)
      at Router.lookup (../../node_modules/find-my-way/index.js:356:14)
      at ../../node_modules/light-my-request/index.js:101:38
      at Request.prepare (../../node_modules/light-my-request/lib/request.js:110:12)
      at ../../node_modules/light-my-request/index.js:101:11
      at doInject (../../node_modules/light-my-request/index.js:97:12)
      at Chain.<computed> [as then] (../../node_modules/light-my-request/index.js:187:23)
          at runMicrotasks (<anonymous>)
```

This fixes the socket object to have an `on` function (as well as the rest of the EventEmitter interface) so that things that add timeout handlers can at least add them without erroring. This doesn't implement actual timeout injection, but I think that could be done in userland by emitting events on the socket manually.

Similar to fastify#99.
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina requested a review from delvedor October 5, 2020 15:02
Copy link
Contributor

@L2jLiga L2jLiga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit e1444d7 into fastify:master Oct 6, 2020
@airhorns airhorns deleted the socket-event-emitter branch October 6, 2020 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants