Pact-js: Pact tests fail when ipv6 is disabled

[tristanHessell]

Software versions

• OS: Ubuntu MATE 18.04.2
• Consumer Pact library: e.g. Pact JS v9.2.0
• Provider Pact library:
• Node Version: v12.4.0

Expected behaviour

The pact tests to run through as expected.

Actual behaviour

Pact Consumer tests are failing at the provider setup level with error listen EADDRNOTAVAIL: address not available ::1:5000.

For clarity, 5000 is the port i have specified in the Pact constructor options.

Steps to reproduce

Make sure that ipv6 is disabled - for me in linux-land this was through /etc/sysctl.conf having the entry net.ipv6.conf.lo.disable_ipv6 = 1.

---

The failures were due to my machine not allowing ipv6 on localhost.

These failures were caused by the addition of testing for the ipv6 localhost address (::1), introduced in here: https://github.com/pact-foundation/pact-js/compare/v9.1.1...v9.2.0 in src/common/net.ts.

I'm raising this not because there is a bug, but:

1. This has inadvertently "broken" between pact releases.
2. If anyone else has the same error, they'll be able to find this issue.

[tristanHessell]

I'm thinking of adding some custom error messaging, similar to how EADDRINUSE is handled in https://github.com/pact-foundation/pact-js/blob/3b39a8e0875989963dc420f070bb3a88864a7f54/src/common/net.ts.

Does that sound acceptable?

[TimothyJones]

That sounds like a clear improvement, yes please. Thanks for raising this!

Sent from my mobile

On 18 Oct 2019, at 5:15 pm, Tristan Hessell [e-mail] wrote:


I'm thinking of adding some custom error messaging, similar to how EADDRINUSE is handled in https://github.com/pact-foundation/pact-js/blob/3b39a8e0875989963dc420f070bb3a88864a7f54/src/common/net.ts.

Does that sound acceptable?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

[mefellows]

My understanding is that you literally can't use pact if IPv6 is disabled and you specify a dedicated port, which is...bad to say the least.

Your suggestion is welcomed and the detailed report muchly appreciated indeed.

I'm left wondering what a good behaviour is in that port check:

1. We could warn if one of the interfaces is not available but others are, and fail if all promises reject (i.e. all checks fail).
2. We could make the interfaces configurable
3. We could just disable IPv6 entirely

I'm sure there are others too. Thoughts?

[tristanHessell]

@mefellows Is there a reason why pact is checking the different hosts as a part of port availability?

Could it not just check the host that is specified in the pact constructor (or the default host of 127.0.0.1)?

Or to ask in a different way: Why are we testing the availability of hosts that the user isn't using?

[mefellows]

@tristanHessell I definitely understand why that question may have come up and should have addressed it in my previous comment. In my experience, people don't understand the difference between the binding options. I've seen a hosted of bugs (reported / otherwise) where this is the case. e.g. the default host option is 127.0.0.1, which means if running within a Docker container by default, it won't be accessible outside of the container. Conversely, there are environments where binding to the other addresses will fail also. There is also the whole issue of hostnames.

It seemed safer at the time to just ensure that there was no conflict whatsoever, and then it didn't matter how the test author configured their http client (localhost, ip address etc.).

Or to ask in a different way: Why are we testing the availability of hosts that the user isn't using?

So to answer this - we don't know what they are using.

My preference is your suggestion, but me having been bitten in the past makes me wary.

@TimothyJones probably has something sensible to say here (I recall he did when that code was written way back).

[tristanHessell]

@mefellows Thanks for the in-depth response - that all makes sense.

With that in mind, I'm leaning towards your first suggestion:

We could warn if one of the interfaces is not available but others are, and fail if all promises reject (i.e. all checks fail).

I think the above will give the smoothest user experience without the user having to understand binding/hostnames etc.

[mefellows]

Cool. I'll wait for Tim to respond so there is an extra sanity check, but that would certainly be fairly easy to achieve.

Thanks for starting the conversation.

[deekayw0n]

Thanks @tristanHessell for filing! Working in an environment where pipelines are run in a centrally managed CI-CD docker agent and in that agent localhost is configured as both 127.0.0.1 and ::1, and no matter config attempts to bind pact mock svr to 127.0.0.1, cannot get beyond this error message. We've needed to revert to a much older pact version [which fails npm audit due to earlier lodash dependencies] which isn't ideal.

Look forward to a resolution in near-term that supports disabling ipv6 within pact config, specific ipv4 host binding, or "just works" implementation that does not require a kernel runtime config change [as was the case in the past]. Thanks for all that you do!

[TimothyJones]

Thanks for the prompt, @mefellows - I think this is the discussion we had: https://github.com/pact-foundation/pact-js/pull/266

We could warn if one of the interfaces is not available but others are, and fail if all promises reject (i.e. all checks fail).

I like this the most.

I'm still uncertain what the best behaviour is for default addresses to listen on, but I think that's unrelated to this case.

@deekayw0n - I'm not 100% clear on your scenario. Would this warning change solve your issue, too?

[deekayw0n]

@TimothyJones - warning change should allow standard ipv4 binding regardless of ipv6 status—expect this will unblock us! Thanks

[tristanHessell]

@mefellows @TimothyJones I'm happy to try my hand at fixing this - take the pressure you all as maintainers.

I'll have a PR within the next couple of days

[mefellows]

That'd be ace @tristanHessell!

[cbonaco1]

hey all, any idea when the PR for this fix will be ready?

also FYI to anyone who updated to version 9.2.2 of pact due to npm audit vulnerabilities, version 9.1.1 does not report any npm audit vulnerabilities, and does not check for ipv6 addresses.

[mefellows]

Well, we're working on it. You can follow our progress here https://github.com/pact-foundation/pact-js/pull/389

[TimothyJones]

We're releasing 9.2.3 today with this fix. Thanks to everyone who participated, and many thanks to @tristanHessell for the implementation.