Skip to content

Commit 1ab4903

Browse files
committed
Merge branch 'hotfix/exception-handling'
Better exception handling - Any exception captured by the `dispatch.error` listener is pushed into the created `ApiProblem`, thus ensuring that we will get a stack trace if `display_exceptions` is enabled. - Traces are now captured to a `trace` member of the problem, using `getTrace()` instead of `getTraceAsString()` - Previous exceptions are now captured to an `exception_stack` member, as an array of objects with `code`, `message`, and `trace` members.
2 parents c518e0e + 2fd97c9 commit 1ab4903

File tree

5 files changed

+40
-23
lines changed

5 files changed

+40
-23
lines changed

src/ZF/ApiProblem/ApiProblem.php

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -296,14 +296,24 @@ protected function createDetailFromException()
296296
return $e->getMessage();
297297
}
298298

299-
$message = '';
300-
do {
301-
$message .= $e->getMessage() . "\n";
302-
$message .= $e->getTraceAsString() . "\n";
299+
$message = trim($e->getMessage());
300+
$this->additionalDetails['trace'] = $e->getTrace();
301+
302+
$previous = array();
303+
$e = $e->getPrevious();
304+
while ($e) {
305+
$previous[] = array(
306+
'code' => (int) $e->getCode(),
307+
'message' => trim($e->getMessage()),
308+
'trace' => $e->getTrace(),
309+
);
303310
$e = $e->getPrevious();
304-
} while ($e instanceof \Exception);
311+
}
312+
if (count($previous)) {
313+
$this->additionalDetails['exception_stack'] = $previous;
314+
}
305315

306-
return trim($message);
316+
return $message;
307317
}
308318

309319
/**

src/ZF/ApiProblem/Listener/ApiProblemListener.php

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,7 @@ public function onDispatchError(MvcEvent $e)
164164
if (0 === $status) {
165165
$status = 500;
166166
}
167-
$detail = $exception->getMessage();
168-
$problem = new ApiProblem($status, $detail);
167+
$problem = new ApiProblem($status, $exception);
169168
} else {
170169
// If it's not an exception, do not know what to do.
171170
return;
@@ -179,8 +178,8 @@ public function onDispatchError(MvcEvent $e)
179178

180179
/**
181180
* Determine if we have a valid error event
182-
*
183-
* @param MvcEvent $e
181+
*
182+
* @param MvcEvent $e
184183
* @return bool
185184
*/
186185
protected function validateErrorEvent(MvcEvent $e)

test/ZFTest/ApiProblem/ApiProblemTest.php

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,30 +65,35 @@ public function testExceptionMessageIsUsedForDetail()
6565
$this->assertEquals($exception->getMessage(), $payload['detail']);
6666
}
6767

68-
public function testDetailCanIncludeStackTrace()
68+
public function testExceptionsCanTriggerInclusionOfStackTraceInDetails()
6969
{
7070
$exception = new \Exception('exception message');
7171
$apiProblem = new ApiProblem('500', $exception);
7272
$apiProblem->setDetailIncludesStackTrace(true);
7373
$payload = $apiProblem->toArray();
74-
$this->assertArrayHasKey('detail', $payload);
75-
$this->assertEquals($exception->getMessage() . "\n" . $exception->getTraceAsString(), $payload['detail']);
74+
$this->assertArrayHasKey('trace', $payload);
75+
$this->assertInternalType('array', $payload['trace']);
76+
$this->assertEquals($exception->getTrace(), $payload['trace']);
7677
}
7778

78-
public function testDetailCanIncludeNestedExceptions()
79+
public function testExceptionsCanTriggerInclusionOfNestedExceptions()
7980
{
8081
$exceptionChild = new \Exception('child exception');
8182
$exceptionParent = new \Exception('parent exception', null, $exceptionChild);
8283

8384
$apiProblem = new ApiProblem('500', $exceptionParent);
8485
$apiProblem->setDetailIncludesStackTrace(true);
8586
$payload = $apiProblem->toArray();
86-
$this->assertArrayHasKey('detail', $payload);
87-
$expected = $exceptionParent->getMessage() . "\n"
88-
. $exceptionParent->getTraceAsString() . "\n"
89-
. $exceptionChild->getMessage() . "\n"
90-
. $exceptionChild->getTraceAsString();
91-
$this->assertEquals($expected, $payload['detail']);
87+
$this->assertArrayHasKey('exception_stack', $payload);
88+
$this->assertInternalType('array', $payload['exception_stack']);
89+
$expected = array(
90+
array(
91+
'code' => $exceptionChild->getCode(),
92+
'message' => $exceptionChild->getMessage(),
93+
'trace' => $exceptionChild->getTrace(),
94+
),
95+
);
96+
$this->assertEquals($expected, $payload['exception_stack']);
9297
}
9398

9499
public function testProblemTypeUrlIsUsedVerbatim()

test/ZFTest/ApiProblem/Listener/SendApiProblemResponseListenerTest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,9 @@ public function testEnablingDisplayExceptionFlagRendersExceptionStackTrace()
6767
$contents = ob_get_clean();
6868
$this->assertInternalType('string', $contents);
6969
$data = json_decode($contents, true);
70-
$this->assertContains("\n", $data['detail']);
71-
$this->assertContains($this->exception->getTraceAsString(), $data['detail']);
70+
$this->assertArrayHasKey('trace', $data);
71+
$this->assertInternalType('array', $data['trace']);
72+
$this->assertGreaterThanOrEqual(1, count($data['trace']));
7273
}
7374

7475
public function testSendContentDoesNothingIfEventDoesNotContainApiProblemResponse()

test/ZFTest/ApiProblem/View/ApiProblemRendererTest.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ public function testCanHintToApiProblemToRenderStackTrace()
4545
$this->renderer->setDisplayExceptions(true);
4646
$test = $this->renderer->render($model);
4747
$test = json_decode($test, true);
48-
$this->assertContains($exception->getMessage() . "\n" . $exception->getTraceAsString(), $test['detail']);
48+
$this->assertArrayHasKey('trace', $test);
49+
$this->assertInternalType('array', $test['trace']);
50+
$this->assertGreaterThanOrEqual(1, count($test['trace']));
4951
}
5052
}

0 commit comments

Comments
 (0)