update socket class with missing address method

[kvernon]

Context

currently, I found nock and am trying to test out a http module flow. along the way, we require extracting the address() result object.

w/in the response, we have this event:

req.on( 'socket', ( socket ) => {
      console.log(socket);
      req[ '_socketAddr' ] = socket.address();
} );

What are you trying to do and how would you want to do it differently? Is it something you currently you cannot do? Is this related to an issue/problem?

In the case of our tests, because the method doesn't exist, the test throws an exception/error, which causes the test to fail.

Alternatives
I'd love to know of alternatives: like could I assign a mock/stub socket to contain that address method?

Can you achieve the same result doing it in an alternative way? Is the alternative considerable?

Currently I know no way to make this happen

Has the feature been requested before?

I do no think so

If the feature request is accepted, would you be willing to submit a PR?

Yes, I can give it a whirl :)

[paulmelnikow]

This sounds like a good feature to add to our mock Socket class. There was a recent PR adding the unref method (#1612). It may be helpful to include a link to the reference docs.

By the way, what would you have the mock function return?

[kvernon]

Howdy @paulmelnikow ,

Thanks for following up with me. I meant to include a the actual reference to to the address() method: https://nodejs.org/dist/latest-v10.x/docs/api/net.html#net_socket_address, which returns this object (per example) { port: 12346, family: 'IPv4', address: '127.0.0.1' }.

My guess, and I'm not 100% here, is that they are from remoteAddress, remotePort, and remoteFamily, so I would store them on the object to align w/ the Socket object shame (your call).

The idea was through options pass in address/ip and port. From there, it would probably be providing defaults for the ip because this is mocking (my thinking here).

Thoughts on that -- or did I get way into the trenches here?

[mastermatt]

The options param is meant to take the same values as http.request, which already (optionally) takes a port and family. But I have mixed feelings about adding new options like address. Would it be wrong to just hardcode 127.0.0.1?

Off the top of my head, something like this?

constructor(options) {
  // ... existing stuff
  this.remotePort = parseInt(options.port) || this.authorized ? 443 : 80
  this.remoteFamily = options.family === 6 ? 'IPv6' : 'IPv4'
  this.remoteAddress = options.family === 6 ? '::1' :  '127.0.0.1'
}

address() {
  return {
    port: this.remotePort,
    family: this.remoteFamily,
    address: this.remoteAddress
  }
}

Docs for the new attrs too.

[kvernon]

Your idea looks amazing to me! 😃

[mastermatt]

@kvernon do you want to pick this up and add tests?

[kvernon]

It's been a little confusing working with TAP, but I feel like it's getting better. Right now, I can't quite understand what's going on with a failing test in a different area...

[mastermatt]

tap is rough. There is a convo going in another issue, about moving away from it.
I tend to copy the body of the test I'm working on into a scratch file and run it directly.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We try to do our best, but nock is maintained by volunteers and there is only so much we can do at a time. Thank you for your contributions.

[nockbot]

🎉 This issue has been resolved in version 11.7.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀