search

pelias / fuzzy-tester

A fuzzy testing library for geocoding
https://pelias.io
4 stars 6 forks source link

fix inifinte loop with invalid hostname #212

Closed arnesetzer closed 3 months ago

arnesetzer commented 4 months ago

:wave: 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 :rocket:

Here's what actually got changed :clap:

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 :eyes:

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.

missinglink commented 4 months ago

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 else { as the throw precludes the need for an else?

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 🙏

arnesetzer commented 4 months ago

Is more readable now. Removed unused else statement. And you were right, there was a brace to much. 🙈

orangejulius commented 4 months ago

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 request or possibly with a call to url.parse.

arnesetzer commented 3 months ago

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 console.err( err.hostname ) before the error? The node stack trace inflates the exception somewhat.😅