Skip to content

Commit d8f748f

Browse files
committed
Improve error handling when sending data to DNS server fails (macOS)
1 parent 900bb63 commit d8f748f

3 files changed

Lines changed: 56 additions & 2 deletions

File tree

.github/workflows/ci.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,19 @@ jobs:
3333
- run: vendor/bin/phpunit --coverage-text -c phpunit.xml.legacy
3434
if: ${{ matrix.php < 7.3 }}
3535

36+
PHPUnit-macOS:
37+
name: PHPUnit (macOS)
38+
runs-on: macos-10.15
39+
continue-on-error: true
40+
steps:
41+
- uses: actions/checkout@v2
42+
- uses: shivammathur/setup-php@v2
43+
with:
44+
php-version: 8.0
45+
coverage: xdebug
46+
- run: composer install
47+
- run: vendor/bin/phpunit --coverage-text
48+
3649
PHPUnit-hhvm:
3750
name: PHPUnit (HHVM)
3851
runs-on: ubuntu-18.04

src/Query/UdpTransportExecutor.php

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,20 @@ public function query(Query $query)
137137

138138
// set socket to non-blocking and immediately try to send (fill write buffer)
139139
\stream_set_blocking($socket, false);
140-
\fwrite($socket, $queryData);
140+
$written = @\fwrite($socket, $queryData);
141+
142+
if ($written !== \strlen($queryData)) {
143+
// Write may potentially fail, but most common errors are already caught by connection check above.
144+
// Among others, macOS is known to report here when trying to send to broadcast address.
145+
// @codeCoverageIgnoreStart
146+
$error = \error_get_last();
147+
\preg_match('/errno=(\d+) (.+)/', $error['message'], $m);
148+
return \React\Promise\reject(new \RuntimeException(
149+
'DNS query for ' . $query->name . ' failed: Unable to send query to DNS server (' . (isset($m[2]) ? $m[2] : $error['message']) . ')',
150+
isset($m[1]) ? (int) $m[1] : 0
151+
));
152+
// @codeCoverageIgnoreEnd
153+
}
141154

142155
$loop = $this->loop;
143156
$deferred = new Deferred(function () use ($loop, $socket, $query) {

tests/Query/UdpTransportExecutorTest.php

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,35 @@ public function testQueryRejectsIfServerConnectionFails()
120120
$promise = $executor->query($query);
121121

122122
$this->assertInstanceOf('React\Promise\PromiseInterface', $promise);
123-
$promise->then(null, $this->expectCallableOnce());
123+
124+
$exception = null;
125+
$promise->then(null, function ($reason) use (&$exception) {
126+
$exception = $reason;
127+
});
128+
129+
$this->setExpectedException('RuntimeException', 'Unable to connect to DNS server (Failed to parse address "///")');
130+
throw $exception;
131+
}
132+
133+
public function testQueryRejectsIfSendToServerFailsAfterConnection()
134+
{
135+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
136+
$loop->expects($this->never())->method('addReadStream');
137+
138+
$executor = new UdpTransportExecutor('255.255.255.255', $loop);
139+
140+
$query = new Query('google.com', Message::TYPE_A, Message::CLASS_IN);
141+
$promise = $executor->query($query);
142+
143+
$this->assertInstanceOf('React\Promise\PromiseInterface', $promise);
144+
145+
$exception = null;
146+
$promise->then(null, function ($reason) use (&$exception) {
147+
$exception = $reason;
148+
});
149+
150+
$this->setExpectedException('RuntimeException', 'to DNS server (Permission denied)', SOCKET_EACCES);
151+
throw $exception;
124152
}
125153

126154
/**

0 commit comments

Comments
 (0)