Skip to content

Commit 9050309

Browse files
WyriHaximusWyriHaximus
authored
Merge pull request #284 from clue-labs/trace
Fix invalid references in exception stack trace
2 parents f474156 + e01f93d commit 9050309

File tree

4 files changed

+151
-90
lines changed

4 files changed

+151
-90
lines changed

‎src/DnsConnector.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,11 @@ function ($resolve, $reject) use (&$promise, &$resolved, $uri, $connector, $host
7373

7474
// Exception trace arguments are not available on some PHP 7.4 installs
7575
// @codeCoverageIgnoreStart
76-
foreach ($trace as &$one) {
76+
foreach ($trace as $ti => $one) {
7777
if (isset($one['args'])) {
78-
foreach ($one['args'] as &$arg) {
78+
foreach ($one['args'] as $ai => $arg) {
7979
if ($arg instanceof \Closure) {
80-
$arg = 'Object(' . \get_class($arg) . ')';
80+
$trace[$ti]['args'][$ai] = 'Object(' . \get_class($arg) . ')';
8181
}
8282
}
8383
}

‎src/SecureConnector.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public function connect($uri)
4343
$context = $this->context;
4444
$encryption = $this->streamEncryption;
4545
$connected = false;
46+
/** @var \React\Promise\PromiseInterface $promise */
4647
$promise = $this->connector->connect(
4748
\str_replace('tls://', '', $uri)
4849
)->then(function (ConnectionInterface $connection) use ($context, $encryption, $uri, &$promise, &$connected) {
@@ -86,11 +87,11 @@ public function connect($uri)
8687

8788
// Exception trace arguments are not available on some PHP 7.4 installs
8889
// @codeCoverageIgnoreStart
89-
foreach ($trace as &$one) {
90+
foreach ($trace as $ti => $one) {
9091
if (isset($one['args'])) {
91-
foreach ($one['args'] as &$arg) {
92+
foreach ($one['args'] as $ai => $arg) {
9293
if ($arg instanceof \Closure) {
93-
$arg = 'Object(' . \get_class($arg) . ')';
94+
$trace[$ti]['args'][$ai] = 'Object(' . \get_class($arg) . ')';
9495
}
9596
}
9697
}

‎tests/DnsConnectorTest.php‎

Lines changed: 77 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,18 @@ public function testConnectRejectsIfGivenIpAndTcpConnectorRejectsWithRuntimeExce
9292
$this->tcp->expects($this->once())->method('connect')->with('1.2.3.4:80')->willReturn($promise);
9393

9494
$promise = $this->connector->connect('1.2.3.4:80');
95-
$promise->cancel();
9695

97-
$this->setExpectedException('RuntimeException', 'Connection to tcp://1.2.3.4:80 failed: Connection failed', 42);
98-
$this->throwRejection($promise);
96+
$exception = null;
97+
$promise->then(null, function ($reason) use (&$exception) {
98+
$exception = $reason;
99+
});
100+
101+
assert($exception instanceof \RuntimeException);
102+
$this->assertInstanceOf('RuntimeException', $exception);
103+
$this->assertEquals('Connection to tcp://1.2.3.4:80 failed: Connection failed', $exception->getMessage());
104+
$this->assertEquals(42, $exception->getCode());
105+
$this->assertNull($exception->getPrevious());
106+
$this->assertNotEquals('', $exception->getTraceAsString());
99107
}
100108

101109
public function testConnectRejectsIfGivenIpAndTcpConnectorRejectsWithInvalidArgumentException()
@@ -105,10 +113,18 @@ public function testConnectRejectsIfGivenIpAndTcpConnectorRejectsWithInvalidArgu
105113
$this->tcp->expects($this->once())->method('connect')->with('1.2.3.4:80')->willReturn($promise);
106114

107115
$promise = $this->connector->connect('1.2.3.4:80');
108-
$promise->cancel();
109116

110-
$this->setExpectedException('InvalidArgumentException', 'Invalid', 42);
111-
$this->throwRejection($promise);
117+
$exception = null;
118+
$promise->then(null, function ($reason) use (&$exception) {
119+
$exception = $reason;
120+
});
121+
122+
assert($exception instanceof \InvalidArgumentException);
123+
$this->assertInstanceOf('InvalidArgumentException', $exception);
124+
$this->assertEquals('Invalid', $exception->getMessage());
125+
$this->assertEquals(42, $exception->getCode());
126+
$this->assertNull($exception->getPrevious());
127+
$this->assertNotEquals('', $exception->getTraceAsString());
112128
}
113129

114130
public function testConnectRejectsWithOriginalHostnameInMessageAfterResolvingIfTcpConnectorRejectsWithRuntimeException()
@@ -118,10 +134,18 @@ public function testConnectRejectsWithOriginalHostnameInMessageAfterResolvingIfT
118134
$this->tcp->expects($this->once())->method('connect')->with('1.2.3.4:80?hostname=example.com')->willReturn($promise);
119135

120136
$promise = $this->connector->connect('example.com:80');
121-
$promise->cancel();
122137

123-
$this->setExpectedException('RuntimeException', 'Connection to tcp://example.com:80 failed: Connection to tcp://1.2.3.4:80 failed: Connection failed', 42);
124-
$this->throwRejection($promise);
138+
$exception = null;
139+
$promise->then(null, function ($reason) use (&$exception) {
140+
$exception = $reason;
141+
});
142+
143+
assert($exception instanceof \RuntimeException);
144+
$this->assertInstanceOf('RuntimeException', $exception);
145+
$this->assertEquals('Connection to tcp://example.com:80 failed: Connection to tcp://1.2.3.4:80 failed: Connection failed', $exception->getMessage());
146+
$this->assertEquals(42, $exception->getCode());
147+
$this->assertInstanceOf('RuntimeException', $exception->getPrevious());
148+
$this->assertNotEquals('', $exception->getTraceAsString());
125149
}
126150

127151
public function testConnectRejectsWithOriginalExceptionAfterResolvingIfTcpConnectorRejectsWithInvalidArgumentException()
@@ -131,10 +155,18 @@ public function testConnectRejectsWithOriginalExceptionAfterResolvingIfTcpConnec
131155
$this->tcp->expects($this->once())->method('connect')->with('1.2.3.4:80?hostname=example.com')->willReturn($promise);
132156

133157
$promise = $this->connector->connect('example.com:80');
134-
$promise->cancel();
135158

136-
$this->setExpectedException('InvalidArgumentException', 'Invalid', 42);
137-
$this->throwRejection($promise);
159+
$exception = null;
160+
$promise->then(null, function ($reason) use (&$exception) {
161+
$exception = $reason;
162+
});
163+
164+
assert($exception instanceof \InvalidArgumentException);
165+
$this->assertInstanceOf('InvalidArgumentException', $exception);
166+
$this->assertEquals('Invalid', $exception->getMessage());
167+
$this->assertEquals(42, $exception->getCode());
168+
$this->assertNull($exception->getPrevious());
169+
$this->assertNotEquals('', $exception->getTraceAsString());
138170
}
139171

140172
public function testSkipConnectionIfDnsFails()
@@ -145,8 +177,17 @@ public function testSkipConnectionIfDnsFails()
145177

146178
$promise = $this->connector->connect('example.invalid:80');
147179

148-
$this->setExpectedException('RuntimeException', 'Connection to tcp://example.invalid:80 failed during DNS lookup: DNS error');
149-
$this->throwRejection($promise);
180+
$exception = null;
181+
$promise->then(null, function ($reason) use (&$exception) {
182+
$exception = $reason;
183+
});
184+
185+
assert($exception instanceof \RuntimeException);
186+
$this->assertInstanceOf('RuntimeException', $exception);
187+
$this->assertEquals('Connection to tcp://example.invalid:80 failed during DNS lookup: DNS error', $exception->getMessage());
188+
$this->assertEquals(0, $exception->getCode());
189+
$this->assertInstanceOf('RuntimeException', $exception->getPrevious());
190+
$this->assertNotEquals('', $exception->getTraceAsString());
150191
}
151192

152193
public function testRejectionExceptionUsesPreviousExceptionIfDnsFails()
@@ -171,12 +212,17 @@ public function testCancelDuringDnsCancelsDnsAndDoesNotStartTcpConnection()
171212
$promise = $this->connector->connect('example.com:80');
172213
$promise->cancel();
173214

174-
$this->setExpectedException(
175-
'RuntimeException',
176-
'Connection to tcp://example.com:80 cancelled during DNS lookup (ECONNABORTED)',
177-
defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103
178-
);
179-
$this->throwRejection($promise);
215+
$exception = null;
216+
$promise->then(null, function ($reason) use (&$exception) {
217+
$exception = $reason;
218+
});
219+
220+
assert($exception instanceof \RuntimeException);
221+
$this->assertInstanceOf('RuntimeException', $exception);
222+
$this->assertEquals('Connection to tcp://example.com:80 cancelled during DNS lookup (ECONNABORTED)', $exception->getMessage());
223+
$this->assertEquals(defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103, $exception->getCode());
224+
$this->assertNull($exception->getPrevious());
225+
$this->assertNotEquals('', $exception->getTraceAsString());
180226
}
181227

182228
public function testCancelDuringTcpConnectionCancelsTcpConnectionIfGivenIp()
@@ -216,12 +262,17 @@ public function testCancelDuringTcpConnectionCancelsTcpConnectionWithTcpRejectio
216262

217263
$promise->cancel();
218264

219-
$this->setExpectedException(
220-
'RuntimeException',
221-
'Connection cancelled',
222-
defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103
223-
);
224-
$this->throwRejection($promise);
265+
$exception = null;
266+
$promise->then(null, function ($reason) use (&$exception) {
267+
$exception = $reason;
268+
});
269+
270+
assert($exception instanceof \RuntimeException);
271+
$this->assertInstanceOf('RuntimeException', $exception);
272+
$this->assertEquals('Connection to tcp://example.com:80 failed: Connection cancelled', $exception->getMessage());
273+
$this->assertEquals(defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103, $exception->getCode());
274+
$this->assertInstanceOf('RuntimeException', $exception->getPrevious());
275+
$this->assertNotEquals('', $exception->getTraceAsString());
225276
}
226277

227278
public function testRejectionDuringDnsLookupShouldNotCreateAnyGarbageReferences()
@@ -336,14 +387,4 @@ public function testCancelDuringTcpConnectionShouldNotCreateAnyGarbageReferences
336387

337388
$this->assertEquals(0, gc_collect_cycles());
338389
}
339-
340-
private function throwRejection($promise)
341-
{
342-
$ex = null;
343-
$promise->then(null, function ($e) use (&$ex) {
344-
$ex = $e;
345-
});
346-
347-
throw $ex;
348-
}
349390
}

‎tests/SecureConnectorTest.php‎

Lines changed: 67 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,18 @@ public function testConnectWillRejectWithTlsUriWhenUnderlyingConnectorRejects()
8080
)));
8181

8282
$promise = $this->connector->connect('example.com:80');
83-
$promise->cancel();
8483

85-
$this->setExpectedException(
86-
'RuntimeException',
87-
'Connection to tls://example.com:80 failed: Connection refused (ECONNREFUSED)',
88-
defined('SOCKET_ECONNREFUSED') ? SOCKET_ECONNREFUSED : 111
89-
);
90-
$this->throwRejection($promise);
84+
$exception = null;
85+
$promise->then(null, function ($reason) use (&$exception) {
86+
$exception = $reason;
87+
});
88+
89+
assert($exception instanceof \RuntimeException);
90+
$this->assertInstanceOf('RuntimeException', $exception);
91+
$this->assertEquals('Connection to tls://example.com:80 failed: Connection refused (ECONNREFUSED)', $exception->getMessage());
92+
$this->assertEquals(defined('SOCKET_ECONNREFUSED') ? SOCKET_ECONNREFUSED : 111, $exception->getCode());
93+
$this->assertInstanceOf('RuntimeException', $exception->getPrevious());
94+
$this->assertNotEquals('', $exception->getTraceAsString());
9195
}
9296

9397
public function testConnectWillRejectWithOriginalMessageWhenUnderlyingConnectorRejectsWithInvalidArgumentException()
@@ -98,14 +102,18 @@ public function testConnectWillRejectWithOriginalMessageWhenUnderlyingConnectorR
98102
)));
99103

100104
$promise = $this->connector->connect('example.com:80');
101-
$promise->cancel();
102105

103-
$this->setExpectedException(
104-
'InvalidArgumentException',
105-
'Invalid',
106-
42
107-
);
108-
$this->throwRejection($promise);
106+
$exception = null;
107+
$promise->then(null, function ($reason) use (&$exception) {
108+
$exception = $reason;
109+
});
110+
111+
assert($exception instanceof \InvalidArgumentException);
112+
$this->assertInstanceOf('InvalidArgumentException', $exception);
113+
$this->assertEquals('Invalid', $exception->getMessage());
114+
$this->assertEquals(42, $exception->getCode());
115+
$this->assertNull($exception->getPrevious());
116+
$this->assertNotEquals('', $exception->getTraceAsString());
109117
}
110118

111119
public function testCancelDuringTcpConnectionCancelsTcpConnection()
@@ -128,12 +136,17 @@ public function testCancelDuringTcpConnectionCancelsTcpConnectionAndRejectsWithT
128136
$promise = $this->connector->connect('example.com:80');
129137
$promise->cancel();
130138

131-
$this->setExpectedException(
132-
'RuntimeException',
133-
'Connection to tls://example.com:80 cancelled (ECONNABORTED)',
134-
defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103
135-
);
136-
$this->throwRejection($promise);
139+
$exception = null;
140+
$promise->then(null, function ($reason) use (&$exception) {
141+
$exception = $reason;
142+
});
143+
144+
assert($exception instanceof \RuntimeException);
145+
$this->assertInstanceOf('RuntimeException', $exception);
146+
$this->assertEquals('Connection to tls://example.com:80 cancelled (ECONNABORTED)', $exception->getMessage());
147+
$this->assertEquals(defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103, $exception->getCode());
148+
$this->assertInstanceOf('RuntimeException', $exception->getPrevious());
149+
$this->assertNotEquals('', $exception->getTraceAsString());
137150
}
138151

139152
public function testConnectionWillBeClosedAndRejectedIfConnectionIsNoStream()
@@ -145,8 +158,17 @@ public function testConnectionWillBeClosedAndRejectedIfConnectionIsNoStream()
145158

146159
$promise = $this->connector->connect('example.com:80');
147160

148-
$this->setExpectedException('UnexpectedValueException', 'Base connector does not use internal Connection class exposing stream resource');
149-
$this->throwRejection($promise);
161+
$exception = null;
162+
$promise->then(null, function ($reason) use (&$exception) {
163+
$exception = $reason;
164+
});
165+
166+
assert($exception instanceof \UnexpectedValueException);
167+
$this->assertInstanceOf('UnexpectedValueException', $exception);
168+
$this->assertEquals('Base connector does not use internal Connection class exposing stream resource', $exception->getMessage());
169+
$this->assertEquals(0, $exception->getCode());
170+
$this->assertNull($exception->getPrevious());
171+
$this->assertNotEquals('', $exception->getTraceAsString());
150172
}
151173

152174
public function testStreamEncryptionWillBeEnabledAfterConnecting()
@@ -160,10 +182,9 @@ public function testStreamEncryptionWillBeEnabledAfterConnecting()
160182
$ref->setAccessible(true);
161183
$ref->setValue($this->connector, $encryption);
162184

163-
$pending = new Promise\Promise(function () { }, function () { throw new \RuntimeException('Connection cancelled'); });
164185
$this->tcp->expects($this->once())->method('connect')->with($this->equalTo('example.com:80'))->willReturn(Promise\resolve($connection));
165186

166-
$promise = $this->connector->connect('example.com:80');
187+
$this->connector->connect('example.com:80');
167188
}
168189

169190
public function testConnectionWillBeRejectedIfStreamEncryptionFailsAndClosesConnection()
@@ -178,18 +199,21 @@ public function testConnectionWillBeRejectedIfStreamEncryptionFailsAndClosesConn
178199
$ref->setAccessible(true);
179200
$ref->setValue($this->connector, $encryption);
180201

181-
$pending = new Promise\Promise(function () { }, function () { throw new \RuntimeException('Connection cancelled'); });
182202
$this->tcp->expects($this->once())->method('connect')->with($this->equalTo('example.com:80'))->willReturn(Promise\resolve($connection));
183203

184204
$promise = $this->connector->connect('example.com:80');
185205

186-
try {
187-
$this->throwRejection($promise);
188-
} catch (\RuntimeException $e) {
189-
$this->assertEquals('Connection to tls://example.com:80 failed during TLS handshake: TLS error', $e->getMessage());
190-
$this->assertEquals(123, $e->getCode());
191-
$this->assertNull($e->getPrevious());
192-
}
206+
$exception = null;
207+
$promise->then(null, function ($reason) use (&$exception) {
208+
$exception = $reason;
209+
});
210+
211+
assert($exception instanceof \RuntimeException);
212+
$this->assertInstanceOf('RuntimeException', $exception);
213+
$this->assertEquals('Connection to tls://example.com:80 failed during TLS handshake: TLS error', $exception->getMessage());
214+
$this->assertEquals(123, $exception->getCode());
215+
$this->assertNull($exception->getPrevious());
216+
$this->assertNotEquals('', $exception->getTraceAsString());
193217
}
194218

195219
public function testCancelDuringStreamEncryptionCancelsEncryptionAndClosesConnection()
@@ -212,12 +236,17 @@ public function testCancelDuringStreamEncryptionCancelsEncryptionAndClosesConnec
212236
$promise = $this->connector->connect('example.com:80');
213237
$promise->cancel();
214238

215-
$this->setExpectedException(
216-
'RuntimeException',
217-
'Connection to tls://example.com:80 cancelled during TLS handshake (ECONNABORTED)',
218-
defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103
219-
);
220-
$this->throwRejection($promise);
239+
$exception = null;
240+
$promise->then(null, function ($reason) use (&$exception) {
241+
$exception = $reason;
242+
});
243+
244+
assert($exception instanceof \RuntimeException);
245+
$this->assertInstanceOf('RuntimeException', $exception);
246+
$this->assertEquals('Connection to tls://example.com:80 cancelled during TLS handshake (ECONNABORTED)', $exception->getMessage());
247+
$this->assertEquals(defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103, $exception->getCode());
248+
$this->assertNull($exception->getPrevious());
249+
$this->assertNotEquals('', $exception->getTraceAsString());
221250
}
222251

223252
public function testRejectionDuringConnectionShouldNotCreateAnyGarbageReferences()
@@ -267,14 +296,4 @@ public function testRejectionDuringTlsHandshakeShouldNotCreateAnyGarbageReferenc
267296

268297
$this->assertEquals(0, gc_collect_cycles());
269298
}
270-
271-
private function throwRejection($promise)
272-
{
273-
$ex = null;
274-
$promise->then(null, function ($e) use (&$ex) {
275-
$ex = $e;
276-
});
277-
278-
throw $ex;
279-
}
280299
}

0 commit comments

Comments
 (0)