Exceptions constantly thrown in debugger

[richardpickett]

using "version": "v1.4.0"

Whenever I run my php in the debugger (vscode) and have it catch exceptions, I'm constantly getting this exception:

vendor/react/dns/src/Resolver/Resolver.php:(86)
Exception has occurred.
React\Dns\RecordNotFoundException: DNS query for discord.com did not return a valid answer (NOERROR / NODATA)

This is for a discord bot, so it connects to discord.com.

I run tcpdump and watch the dns requests and response - there are no issues, and I can always resolve the name using dig/host without any problems.

It's near-instantaneous, as soon as I click "run," it's stopped on that exception.

I hear on the discord group that anyone who debugs has this same problem, so I'm happy to dig into this and see if we can resolve it.

It may be the way the dns is invoked, if so, I'd be happy to get that cleared up for the discord coders.

[clue]

@richardpickett Thanks for reporting! We're not currently aware of any such issues, so I'm curious what exactly is being reported here.

How can we best reproduce the problem you're seeing? Is this something that can be reproduced by running any of the examples included in this repository?

If this can not be reproduced by running any of the examples here, I suppose this could be related to reactphp/socket#248 perhaps? Perhaps you can also check if you can reproduce this by running any of the examples included in the Socket component?

Ideally, we would isolate this further without relying on VSCode. I'm not a VSCode user, so I can only guess what your debugger is reporting here. It looks like this might be an internal exception that is correctly reported and properly handled by the Socket component (so it's not an actual error). This would happen because discord.com has only A records (IPv4) and no AAAA records (IPv6) and the Socket component has to check both families to figure out how to create the underlying TCP/IP connection (happy eyeballs).

Are you seeing any problems with this or does it only affect your debugging session?

[richardpickett]

I checked, discord.com doesn't have v6 records.

Is there a way to tell dns to only use v4?

[clue]

That's correct.

As per https://github.com/reactphp/socket#connector, you can temporarily disable the happy eyeballs algorithm like this:

// not recommended!
$connector = new React\Socket\Connector($loop, array(
    'happy_eyeballs' => false
));

Note that this is generally not recommended in case Discord starts supporting IPv6 connections in the future and/or your system has a broken IPv4 or IPv6 setup. Additionally, this makes the connection setup much less reliable, because it will only try a single connection to one of the IPv4 addresses (randomly chosen with no retries). The happy eyeballs algorithm will avoid any of these problems and should be used unless you're absolutely sure you don't need this.

I'm still trying to understand what problem you're seeing exactly, did you make any progress in this regard?

[richardpickett]

More details - it's because HappyEyeBallsConnectionBuilder is the connector which will always try IPv4 first.

How can I tell react to not use HappyEyeBallsConnectionBuilder, and which connector can I use instead?

[clue]

I'm planning to do a longer form blog post about the happy eyeballs algorithm in the future, in the mean time you may want to check out https://clue.engineering/2020/introducing-ipv6-for-reactphp and/or https://en.wikipedia.org/wiki/Happy_Eyeballs. The happy eyeballs algorithm will always check connections over both IPv6 and IPv4 and will automatically select the appropriate transport.

As per https://github.com/reactphp/socket#connector, ReactPHP uses the HappyEyeBallsConnector by default. This is a sane default that works across all systems, no matter whether you have broken IPv4 or IPv6 connectivity.

The same section in the documentation also explains how this can be disabled in case you're sure you only want IPv4 connectivity. Note that this is somewhat limited and should not be used in general, because the happy eyeballs algorithms will properly fall back to IPv4 connections itself and explicitly enforcing this limitation can be problematic for forwards compatibility in case your service ever becomes IPv6-capable.

If you can provide us a gist, we might be able to show you how you can integrate this for your particular code. For example, the React\Http\Browser class accepts an option Reacŧ\Socket\ConnectorInterface as the second argument to control the connector behavior as per https://github.com/reactphp/http#browser.

Again, I'm still trying to understand what problem you're seeing exactly, did you make any progress in this regard? It's my understanding using a different connector might be a work around at best and doesn't really address your underlying issue.

[WyriHaximus]

@richardpickett Which Discord bot is this, and for debugging purposes can you post the output of:

php examples/01-one.php discord.com
php examples/02-concurrent.php discord.com discord.com
php examples/11-all-ips.php discord.com
php examples/91-query-a-and-aaaa.php discord.com

The way we've build the HappyEyeBallsConnector is that it should never error out when you're using the default resolve method, as long as either A or AAAA has records

[richardpickett]

@richardpickett Which Discord bot is this, and for debugging purposes can you post the output of:

It's one that provides stats for Clash Royale.

> php examples/01-one.php discord.com
> php examples/02-concurrent.php discord.com discord.com
> php examples/11-all-ips.php discord.com
> php examples/91-query-a-and-aaaa.php discord.com

I'd be happy to run those, but my composer update pull doesn't include them, and just checking out this repo directly doesn't have the correct autoload setup, so it fails.

The way we've build the HappyEyeBallsConnector is that it should never error out when you're using the default resolve method, as long as either A or AAAA has records

It has A records. To be clear, it's that when it doesn't get an AAAA record it throws two exceptions - which trips the debugger. The resolver code catches the exceptions and handles them, but it's annoying that at the start of the debugging and every couple minutes I have to tell the debugger to continue for two exceptions in the code.

It would be preferable if an exception is thrown only when there's no AAAA and no A.

If you want to know the exact files/lines/exceptions thrown, I can provide them.

Instead, I'm having to initialize the connector with 'happy_eyeballs' => false. Later when discord adopts IPv6, I'll be able to come back and use happy eyeballs.

[WyriHaximus]

The way we've build the HappyEyeBallsConnector is that it should never error out when you're using the default resolve method, as long as either A or AAAA has records

It has A records. To be clear, it's that when it doesn't get an AAAA record it throws two exceptions - which trips the debugger. The resolver code catches the exceptions and handles them, but it's annoying that at the start of the debugging and every couple minutes I have to tell the debugger to continue for two exceptions in the code.

It would be preferable if an exception is thrown only when there's no AAAA and no A.

It uses exceptions internally to deal with mission records, and other errors. They are all caught and dealt with by the HappyEyeBallsConnector. So just to be clear, it stops at the line of the throw statement? Which debugger are you using?

[richardpickett]

So just to be clear, it stops at the line of the throw statement?

Yes, most debuggers will stop at every exception thrown.

My view on exceptions - they aren't "the rule," they are the exception. I'd want my code to handle all "the rule" cases, and throw exceptions when it finds one it doesn't handle, which then I should code for so my code once again handles all the cases.

Of all my debugging, HappyEyeBalls is the first one I've come across that uses exceptions as the way it handles normal cases. It's still common for a site to not have IPv6.

Which debugger are you using?

I'm debugging with vscode with xdebug.

[WyriHaximus]

So just to be clear, it stops at the line of the throw statement?

Yes, most debuggers will stop at every exception thrown.

Ok I get why that's annoying. There are a few different aspects and perspectives at play. Plus I have an idea that will get you the best of both worlds.

Of all my debugging, HappyEyeBalls is the first one I've come across that uses exceptions as the way it handles normal cases. It's still common for a site to not have IPv6.

HappyEyeBalls is a bit special as it will let different connection attempts race. At the start, it will attempt to resolve both A and AAAA at the same time. It will prefer the AAAA records so it waits for a few milliseconds before going ahead with the A records when the AAAA is slow. It will then attempt new connections every X ms, and if the AAAA records come in at some point it will start alternate between AAAA and A until it tried them all. All of this is done to favour IPv6 connections and to create an incentive for network admins with IPv6 support to have big enough bandwidth for it.

Now when there are no AAAA records an exception is tossed because there is an error while resolving. And as an error, it is handled as such with an exception. That is later discarded when an IPv4 connection succeeds. Which is a very normal way to approach this.

Which debugger are you using?

I'm debugging with vscode with xdebug.

There are a few ways of tackling this, the easiest is to just smash the run button. I'm pretty sure xdebug supports ignoring files during debugging so you could ignore the source of the exception. Thirdly you can set an environment var to control whether HappyEyeBalls is enabled or not so you can disable it during debug, but leave it on when deployed:

// not recommended!
$connector = new React\Socket\Connector($loop, array(
    'happy_eyeballs' => getenv('DISABLE_HAPPY_EYEBALLS') === null
));

[richardpickett]

The fact that there is a HappyEyeballs at all is because we assume some sites we connect with will be IPv6, and some will only be IPv4.

Throwing an exception when meeting an expected condition is annoying.

Why not only throw the exception when you can't find any record you can use to connect? That seems more appropriate.

Imagine writing functions that instead of returning values it threw exceptions to communicate results to the call stack?

Wait. You don't have to imagine it.

🤣

"But it's because of promises" will just be handwaving, there are graceful ways to handle expected conditions without throwing exceptions, even when using promises. Especially considering that the callstack can be completely gone by the time the exception is thrown - the use of exceptions adds more complexity rather than reducing it.

I digress.

I'll have to look and see if I can have the debugger ignore certain files - but it doesn't matter since now I'm just not using HEB at all.

I like your suggestion of using an ENV setting, in my case I'm doing this with json config files, so will control it there.

[clue]

The original report made it sound like there was some error in this project, so let's start with a recap: It looks like there's no error in this project and this is solely boils down to some unfortunate(?) and/or misinterpreted debugging output from VSCode debugger.

Here's a somewhat simplified version of what's going in over several thousand lines of code in interaction between the socket and DNS component:

try {
    throw new RuntimeException('Foo');
} catch (Exception $e) {
    echo $e->getMessage() . PHP_EOL;
}

While we can continue discussing this design (see next paragraph), this code works just fine without causing any problems. Consequently, I would argue that a debugger should not report an error here (I'm not sure what your debugger reports exactly, so without more information I can only guess).

At this point, we can focus on discussing the API design. I agree that vexing exceptions is not exactly a perfect design. The code in question has been introduced by me a couple of years ago (#110), in order to add support for resolving more address types. This new(er) resolveAll() method resembles the much, much older resolve() method semantics and accordingly also throws a RecordNotFoundException when no records could be found. While this is intentionally designed to resemble the existing method signatures, I agree that this could (should?) have returned an empty array in this case instead.

We're happy to accept PRs to address this if you feel it's worth it. Changing this piece of code isn't really a lot of work, but the major hindrance I'm seeing would be the BC break involved. We're committed to providing long-term support (LTS) for our component and at the moment I'm not sold breaking BC would be worth it if we're only discussing code style.

I'm open to discussing this further, but other than that, I believe this has been answered, so I'm closing this for now. Please come back with more details if this problem persists and we can always keep discussing and/or reopen this 👍