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

fix: Handle systems without IPv6 properly #97

Merged
merged 1 commit into from
Oct 18, 2022
Merged

fix: Handle systems without IPv6 properly #97

merged 1 commit into from
Oct 18, 2022

Conversation

danez
Copy link
Contributor

@danez danez commented Oct 18, 2022

This fixes IPv6 again.
On systems that have the IPv6 stack disabled (for example ubuntu) the name resolution won't even work for IPv6 and ENOTFOUND throws.

This PR adds code that handles this case and also disables IPv6.

netlify/cli#5166

I also added some generic error handling for IPv6 on unexpected errors. So if an unexpected error happens while we are using IPv6 we simply ignore the error and disable IPv6.

I tried to come up with some tests but this is really hard to test. I have an idea though, by disabling IPv6 in the github actions image, but I will do that separatly.

@codecov
Copy link

codecov bot commented Oct 18, 2022

Codecov Report

Merging #97 (0f4661b) into main (53ff7fa) will increase coverage by 1.30%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##             main      #97      +/-   ##
==========================================
+ Coverage   90.50%   91.80%   +1.30%     
==========================================
  Files           9        9              
  Lines         179      183       +4     
==========================================
+ Hits          162      168       +6     
+ Misses         17       15       -2     
Impacted Files Coverage Δ
lib/wait-port.js 88.13% <66.66%> (+2.17%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@dwmkerr
Copy link
Owner

dwmkerr commented Oct 18, 2022

This looks great @danez merging and releasing now

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.

2 participants