-
-
Notifications
You must be signed in to change notification settings - Fork 6
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 inifinte loop with invalid hostname #212
Conversation
I'm on mobile so it's hard to tell, is this code valid? Seems to me the braces don't match? Maybe you can delete the It's kinda hard to tell since half of the PR seems to be white spaces changes, can we just keep the whitespace the same to make the diffs easier to read plz 🙏 |
9d67b39
to
1b113f0
Compare
Is more readable now. Removed unused else statement. And you were right, there was a brace to much. 🙈 |
I like this feature. I can also confirm the code was initially broken with a syntax error but is fixed now. I'm ok to merge it but can we possibly add a feature where it prints the hostname that can't be found? It should be findable either directly from the options we pass to |
The last commit adds the hostname to the error message. arne@workstation:~/temp/fuzzy-tester$ ./bin/fuzzy-tester -e fail
Error: getaddrinfo ENOTFOUND what.ever
at GetAddrInfoReqWrap.onlookupall [as oncomplete] (node:dns:118:26) {
errno: -3008,
code: 'ENOTFOUND',
syscall: 'getaddrinfo',
hostname: 'what.ever'
}
/home/arne/temp/fuzzy-tester/lib/request_urls.js:84
throw new Error( 'Invalid hostname: "'+ err.hostname + '". Stopping execution.' );
^
Error: Invalid hostname: "what.ever". Stopping execution.
at Request._callback (/home/arne/temp/fuzzy-tester/lib/request_urls.js:84:17)
at ... Can you find it quickly enough or should it throw a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍
👋 I did some awesome work for the Pelias project and would love for everyone to have a look at it and provide feedback.
Here's the reason for this change 🚀
Here's what actually got changed 👏
Add another check for the error type. If it is "ENOTFOUND" it now will throw an error and abort the execution.
https://github.com/arnesetzer/pelias-fuzzy-tester/blob/fixInvalidHost/lib/request_urls.js#L83
Here's how others can test the changes 👀
Try a invalid hostname. This should now throw a new error at the first occurance and exit instead of trying to reconnect to this host till ctrl+c.