Skip to content

Commit 7c336fe

Browse files
clueclue
committed
Include domain name and query type in all error messages
1 parent 134d424 commit 7c336fe

18 files changed

+214
-66
lines changed

‎examples/92-query-any.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@
7373
break;
7474
default:
7575
// unknown type uses HEX format
76-
$type = 'Type ' . $answer->type;
76+
$type = 'TYPE' . $answer->type;
7777
$data = wordwrap(strtoupper(bin2hex($data)), 2, ' ', true);
7878
}
7979

‎src/Query/CachingExecutor.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ function (Message $message) use ($cache, $id, $that) {
5757
$pending = null;
5858
});
5959
}, function ($_, $reject) use (&$pending, $query) {
60-
$reject(new \RuntimeException('DNS query for ' . $query->name . ' has been cancelled'));
60+
$reject(new \RuntimeException('DNS query for ' . $query->describe() . ' has been cancelled'));
6161
$pending->cancel();
6262
$pending = null;
6363
});

‎src/Query/CoopExecutor.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public function query(Query $query)
8181
$promise->cancel();
8282
$promise = null;
8383
}
84-
throw new \RuntimeException('DNS query for ' . $query->name . ' has been cancelled');
84+
throw new \RuntimeException('DNS query for ' . $query->describe() . ' has been cancelled');
8585
});
8686
}
8787

‎src/Query/Query.php‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace React\Dns\Query;
44

5+
use React\Dns\Model\Message;
6+
57
/**
68
* This class represents a single question in a query/response message
79
*
@@ -39,4 +41,29 @@ public function __construct($name, $type, $class)
3941
$this->type = $type;
4042
$this->class = $class;
4143
}
44+
45+
/**
46+
* Describes the hostname and query type/class for this query
47+
*
48+
* The output format is supposed to be human readable and is subject to change.
49+
* The format is inspired by RFC 3597 when handling unkown types/classes.
50+
*
51+
* @return string "example.com (A)" or "example.com (CLASS0 TYPE1234)"
52+
* @link https://tools.ietf.org/html/rfc3597
53+
*/
54+
public function describe()
55+
{
56+
$class = $this->class !== Message::CLASS_IN ? 'CLASS' . $this->class . ' ' : '';
57+
58+
$type = 'TYPE' . $this->type;
59+
$ref = new \ReflectionClass('React\Dns\Model\Message');
60+
foreach ($ref->getConstants() as $name => $value) {
61+
if ($value === $this->type && \strpos($name, 'TYPE_') === 0) {
62+
$type = \substr($name, 5);
63+
break;
64+
}
65+
}
66+
67+
return $this->name . ' (' . $class . $type . ')';
68+
}
4269
}

‎src/Query/RetryExecutor.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public function tryQuery(Query $query, $retries)
4343
} elseif ($retries <= 0) {
4444
$errorback = null;
4545
$deferred->reject($e = new \RuntimeException(
46-
'DNS query for ' . $query->name . ' failed: too many retries',
46+
'DNS query for ' . $query->describe() . ' failed: too many retries',
4747
0,
4848
$e
4949
));

‎src/Query/TcpTransportExecutor.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ public function query(Query $query)
166166
$length = \strlen($queryData);
167167
if ($length > 0xffff) {
168168
return \React\Promise\reject(new \RuntimeException(
169-
'DNS query for ' . $query->name . ' failed: Query too large for TCP transport'
169+
'DNS query for ' . $query->describe() . ' failed: Query too large for TCP transport'
170170
));
171171
}
172172

@@ -177,7 +177,7 @@ public function query(Query $query)
177177
$socket = @\stream_socket_client($this->nameserver, $errno, $errstr, 0, \STREAM_CLIENT_CONNECT | \STREAM_CLIENT_ASYNC_CONNECT);
178178
if ($socket === false) {
179179
return \React\Promise\reject(new \RuntimeException(
180-
'DNS query for ' . $query->name . ' failed: Unable to connect to DNS server (' . $errstr . ')',
180+
'DNS query for ' . $query->describe() . ' failed: Unable to connect to DNS server (' . $errstr . ')',
181181
$errno
182182
));
183183
}
@@ -214,7 +214,7 @@ public function query(Query $query)
214214
});
215215

216216
$this->pending[$request->id] = $deferred;
217-
$this->names[$request->id] = $query->name;
217+
$this->names[$request->id] = $query->describe();
218218

219219
return $deferred->promise();
220220
}

‎src/Query/TimeoutExecutor.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public function query(Query $query)
2222
{
2323
return Timer\timeout($this->executor->query($query), $this->timeout, $this->loop)->then(null, function ($e) use ($query) {
2424
if ($e instanceof Timer\TimeoutException) {
25-
$e = new TimeoutException(sprintf("DNS query for %s timed out", $query->name), 0, $e);
25+
$e = new TimeoutException(sprintf("DNS query for %s timed out", $query->describe()), 0, $e);
2626
}
2727
throw $e;
2828
});

‎src/Query/UdpTransportExecutor.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ public function query(Query $query)
128128
$queryData = $this->dumper->toBinary($request);
129129
if (isset($queryData[$this->maxPacketSize])) {
130130
return \React\Promise\reject(new \RuntimeException(
131-
'DNS query for ' . $query->name . ' failed: Query too large for UDP transport',
131+
'DNS query for ' . $query->describe() . ' failed: Query too large for UDP transport',
132132
\defined('SOCKET_EMSGSIZE') ? \SOCKET_EMSGSIZE : 90
133133
));
134134
}
@@ -137,7 +137,7 @@ public function query(Query $query)
137137
$socket = @\stream_socket_client($this->nameserver, $errno, $errstr, 0);
138138
if ($socket === false) {
139139
return \React\Promise\reject(new \RuntimeException(
140-
'DNS query for ' . $query->name . ' failed: Unable to connect to DNS server (' . $errstr . ')',
140+
'DNS query for ' . $query->describe() . ' failed: Unable to connect to DNS server (' . $errstr . ')',
141141
$errno
142142
));
143143
}
@@ -154,7 +154,7 @@ public function query(Query $query)
154154
$error = \error_get_last();
155155
\preg_match('/errno=(\d+) (.+)/', $error['message'], $m);
156156
return \React\Promise\reject(new \RuntimeException(
157-
'DNS query for ' . $query->name . ' failed: Unable to send query to DNS server (' . (isset($m[2]) ? $m[2] : $error['message']) . ')',
157+
'DNS query for ' . $query->describe() . ' failed: Unable to send query to DNS server (' . (isset($m[2]) ? $m[2] : $error['message']) . ')',
158158
isset($m[1]) ? (int) $m[1] : 0
159159
));
160160
}
@@ -165,7 +165,7 @@ public function query(Query $query)
165165
$loop->removeReadStream($socket);
166166
\fclose($socket);
167167

168-
throw new CancellationException('DNS query for ' . $query->name . ' has been cancelled');
168+
throw new CancellationException('DNS query for ' . $query->describe() . ' has been cancelled');
169169
});
170170

171171
$max = $this->maxPacketSize;
@@ -198,7 +198,7 @@ public function query(Query $query)
198198

199199
if ($response->tc) {
200200
$deferred->reject(new \RuntimeException(
201-
'DNS query for ' . $query->name . ' failed: The server returned a truncated result for a UDP query',
201+
'DNS query for ' . $query->describe() . ' failed: The server returned a truncated result for a UDP query',
202202
\defined('SOCKET_EMSGSIZE') ? \SOCKET_EMSGSIZE : 90
203203
));
204204
return;

‎src/Resolver/Resolver.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public function extractValues(Query $query, Message $response)
7272
$message = 'Unknown error response code ' . $code;
7373
}
7474
throw new RecordNotFoundException(
75-
'DNS query for ' . $query->name . ' returned an error response (' . $message . ')',
75+
'DNS query for ' . $query->describe() . ' returned an error response (' . $message . ')',
7676
$code
7777
);
7878
}
@@ -83,7 +83,7 @@ public function extractValues(Query $query, Message $response)
8383
// reject if we did not receive a valid answer (domain is valid, but no record for this type could be found)
8484
if (0 === count($addresses)) {
8585
throw new RecordNotFoundException(
86-
'DNS query for ' . $query->name . ' did not return a valid answer (NOERROR / NODATA)'
86+
'DNS query for ' . $query->describe() . ' did not return a valid answer (NOERROR / NODATA)'
8787
);
8888
}
8989

‎tests/FunctionalResolverTest.php‎

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -107,34 +107,63 @@ public function testResolveAllGoogleCaaResolvesWithCache()
107107
*/
108108
public function testResolveInvalidRejects()
109109
{
110-
$ex = $this->callback(function ($param) {
111-
return ($param instanceof RecordNotFoundException && $param->getCode() === Message::RCODE_NAME_ERROR);
112-
});
113-
114110
$promise = $this->resolver->resolve('example.invalid');
115-
$promise->then($this->expectCallableNever(), $this->expectCallableOnceWith($ex));
116111

117112
$this->loop->run();
113+
114+
$exception = null;
115+
$promise->then(null, function ($reason) use (&$exception) {
116+
$exception = $reason;
117+
});
118+
119+
/** @var \React\Dns\RecordNotFoundException $exception */
120+
$this->assertInstanceOf('React\Dns\RecordNotFoundException', $exception);
121+
$this->assertEquals('DNS query for example.invalid (A) returned an error response (Non-Existent Domain / NXDOMAIN)', $exception->getMessage());
122+
$this->assertEquals(Message::RCODE_NAME_ERROR, $exception->getCode());
118123
}
119124

120125
public function testResolveCancelledRejectsImmediately()
121126
{
122127
// max_nesting_level was set to 100 for PHP Versions < 5.4 which resulted in failing test for legacy PHP
123128
ini_set('xdebug.max_nesting_level', 256);
124129

125-
$ex = $this->callback(function ($param) {
126-
return ($param instanceof \RuntimeException && $param->getMessage() === 'DNS query for google.com has been cancelled');
127-
});
128-
129130
$promise = $this->resolver->resolve('google.com');
130-
$promise->then($this->expectCallableNever(), $this->expectCallableOnceWith($ex));
131131
$promise->cancel();
132132

133133
$time = microtime(true);
134134
$this->loop->run();
135135
$time = microtime(true) - $time;
136136

137137
$this->assertLessThan(0.1, $time);
138+
139+
$exception = null;
140+
$promise->then(null, function ($reason) use (&$exception) {
141+
$exception = $reason;
142+
});
143+
144+
/** @var \React\Dns\Query\CancellationException $exception */
145+
$this->assertInstanceOf('React\Dns\Query\CancellationException', $exception);
146+
$this->assertEquals('DNS query for google.com (A) has been cancelled', $exception->getMessage());
147+
}
148+
149+
/**
150+
* @group internet
151+
*/
152+
public function testResolveAllInvalidTypeRejects()
153+
{
154+
$promise = $this->resolver->resolveAll('google.com', Message::TYPE_PTR);
155+
156+
$this->loop->run();
157+
158+
$exception = null;
159+
$promise->then(null, function ($reason) use (&$exception) {
160+
$exception = $reason;
161+
});
162+
163+
/** @var \React\Dns\RecordNotFoundException $exception */
164+
$this->assertInstanceOf('React\Dns\RecordNotFoundException', $exception);
165+
$this->assertEquals('DNS query for google.com (PTR) did not return a valid answer (NOERROR / NODATA)', $exception->getMessage());
166+
$this->assertEquals(0, $exception->getCode());
138167
}
139168

140169
public function testInvalidResolverDoesNotResolveGoogle()

0 commit comments

Comments
 (0)