Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

send pre/post code context for exceptions and errors #76

Merged
merged 4 commits into from
Apr 22, 2016
Merged

send pre/post code context for exceptions and errors #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0be24cc
send pre/post code context for exceptions and errors
Mar 23, 2016
e42332d
mend
Mar 23, 2016
b970484
abstract out the source file reader and test error/exception payloads
Mar 23, 2016
d4bad68
mend
Mar 23, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 69 additions & 9 deletions src/rollbar.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,14 @@ class RollbarNotifier {
public $report_suppressed = false;
public $use_error_reporting = false;
public $proxy = null;
public $include_error_code_context = false;
public $include_exception_code_context = false;

private $config_keys = array('access_token', 'base_api_url', 'batch_size', 'batched', 'branch',
'capture_error_backtraces', 'code_version', 'environment', 'error_sample_rates', 'handler',
'agent_log_location', 'host', 'logger', 'included_errno', 'person', 'person_fn', 'root',
'scrub_fields', 'shift_function', 'timeout', 'report_suppressed', 'use_error_reporting', 'proxy');
'scrub_fields', 'shift_function', 'timeout', 'report_suppressed', 'use_error_reporting', 'proxy',
'include_error_code_context', 'include_exception_code_context');

// cached values for request/server/person data
private $_request_data = null;
Expand All @@ -137,13 +140,17 @@ class RollbarNotifier {
private $_mt_randmax;

private $_curl_ipresolve_supported;


/** @var iSourceFileReader $_source_file_reader */
private $_source_file_reader;

public function __construct($config) {
foreach ($this->config_keys as $key) {
if (isset($config[$key])) {
$this->$key = $config[$key];
}
}
$this->_source_file_reader = new SourceFileReader();

if (!$this->access_token && $this->handler != 'agent') {
$this->log_error('Missing access token');
Expand Down Expand Up @@ -653,24 +660,34 @@ protected function build_exception_trace_chain(Exception $exc, $extra_data = nul
*/
protected function build_exception_frames(Exception $exc) {
$frames = array();

foreach ($exc->getTrace() as $frame) {
$frames[] = array(
$framedata = array(
'filename' => isset($frame['file']) ? $frame['file'] : '<internal>',
'lineno' => isset($frame['line']) ? $frame['line'] : 0,
'method' => $frame['function']
// TODO include args? need to sanitize first.
);
if($this->include_exception_code_context && isset($frame['file']) && isset($frame['line'])) {
$this->add_frame_code_context($frame['file'], $frame['line'], $framedata);
}
$frames[] = $framedata;
}

// rollbar expects most recent call to be last, not first
$frames = array_reverse($frames);

// add top-level file and line to end of the reversed array
$frames[] = array(
'filename' => $exc->getFile(),
'lineno' => $exc->getLine()
$file = $exc->getFile();
$line = $exc->getLine();
$framedata = array(
'filename' => $file,
'lineno' => $line
);
if($this->include_exception_code_context) {
$this->add_frame_code_context($file, $line, $framedata);
}
$frames[] = $framedata;

$this->shift_method($frames);

Expand Down Expand Up @@ -702,23 +719,31 @@ protected function build_error_frames($errfile, $errline) {
continue;
}

$frames[] = array(
$framedata = array(
// Sometimes, file and line are not set. See:
// http://stackoverflow.com/questions/4581969/why-is-debug-backtrace-not-including-line-number-sometimes
'filename' => isset($frame['file']) ? $frame['file'] : "<internal>",
'lineno' => isset($frame['line']) ? $frame['line'] : 0,
'method' => $frame['function']
);
if($this->include_error_code_context && isset($frame['file']) && isset($frame['line'])) {
$this->add_frame_code_context($frame['file'], $frame['line'], $framedata);
}
$frames[] = $framedata;
}

// rollbar expects most recent call last, not first
$frames = array_reverse($frames);

// add top-level file and line to end of the reversed array
$frames[] = array(
$framedata = array(
'filename' => $errfile,
'lineno' => $errline
);
if($this->include_error_code_context) {
$this->add_frame_code_context($errfile, $errline, $framedata);
}
$frames[] = $framedata;

$this->shift_method($frames);

Expand Down Expand Up @@ -982,10 +1007,45 @@ protected function uuid4() {
protected function load_agent_file() {
$this->_agent_log = fopen($this->agent_log_location . '/rollbar-relay.' . getmypid() . '.' . microtime(true) . '.rollbar', 'a');
}

protected function add_frame_code_context($file, $line, array &$framedata) {
$source = $this->get_source_file_reader()->read_as_array($file);
if (is_array($source)) {
$source = str_replace(array("\n", "\t", "\r"), '', $source);
$total = count($source);
$line = $line - 1;
$framedata['code'] = $source[$line];
$offset = 6;
$min = max($line - $offset, 0);
if ($min !== $line) {
$framedata['context']['pre'] = array_slice($source, $min, $line - $min);
}
$max = min($line + $offset, $total);
if ($max !== $line) {
$framedata['context']['post'] = array_slice($source, $line + 1, $max - $line);
}
}
}

protected function get_source_file_reader() { return $this->_source_file_reader; }
}

interface iRollbarLogger {
public function log($level, $msg);
}

class Ratchetio extends Rollbar {}

interface iSourceFileReader {

/**
* @param string $file_path
* @return string[]
*/
public function read_as_array($file_path);
}

class SourceFileReader implements iSourceFileReader {

public function read_as_array($file_path) { return file($file_path); }
}
122 changes: 121 additions & 1 deletion tests/RollbarNotifierTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@ class RollbarNotifierTest extends PHPUnit_Framework_TestCase {
'code_version' => RollbarNotifier::VERSION,
'batched' => false
);

private static $mockErrorFileSource = array(
"<?php\n",
"\n",
"class Foo extends Bar {\n",
"\n",
'public function getBaz($qux) { return $qux; }',
"\n",
"private function getFred() { return 123; }\n",
"}\n"
);

private $_server;

Expand Down Expand Up @@ -472,7 +483,116 @@ public function testServerBranchConfig() {

$this->assertEquals($payload['data']['server']['branch'], 'my-branch');
}


public function testErrorPrePostCodeContextPayloadData() {

// arrange
$mock_error_file_path = '/foo/bar/baz.php';
$mock_error_file_source = self::$mockErrorFileSource;
$payload = null;
$config = self::$simpleConfig;
$config['include_error_code_context'] = true;
$notifier = m::mock('RollbarNotifier[send_payload,get_source_file_reader]', array($config))
->shouldAllowMockingProtectedMethods();
$notifier->shouldReceive('send_payload')->once()
->with(m::on(function($input) use (&$payload) {
$payload = $input;
return true;
}));
$reader = m::mock('SourceFileReader');
$reader->shouldReceive('read_as_array')
->atLeast()
->once()
->andReturnUsing(function($file) use ($mock_error_file_path, $mock_error_file_source) {
if ($file === $mock_error_file_path) {
return $mock_error_file_source;
}
return file($file);
});
$notifier->shouldReceive('get_source_file_reader')
->andReturn($reader);

// act
$notifier->report_php_error(1, 'foo', $mock_error_file_path, 5);

// assert
$mock_error_file_frame = null;
foreach($payload['data']['body']['trace']['frames'] as $frame) {
if($frame['filename'] === $mock_error_file_path) {
$mock_error_file_frame = $frame;
break;
}
}
$this->assertNotNull($mock_error_file_frame);
$this->assertEquals('public function getBaz($qux) { return $qux; }', $mock_error_file_frame['code']);
$this->assertEquals(array(
'<?php',
'',
'class Foo extends Bar {',
''
), $mock_error_file_frame['context']['pre']);
$this->assertEquals(array(
'',
'private function getFred() { return 123; }',
'}'
), $mock_error_file_frame['context']['post']);
}

public function testExceptionPrePostCodeContextPayloadData() {

// arrange
$mock_error_file_path = '/foo/bar/baz.php';
$mock_error_file_source = self::$mockErrorFileSource;
$payload = null;
$config = self::$simpleConfig;
$config['include_exception_code_context'] = true;
$notifier = m::mock('RollbarNotifier[send_payload,get_source_file_reader]', array($config))
->shouldAllowMockingProtectedMethods();
$notifier->shouldReceive('send_payload')->once()
->with(m::on(function($input) use (&$payload) {
$payload = $input;
return true;
}));
$reader = m::mock('SourceFileReader');
$reader->shouldReceive('read_as_array')
->atLeast()
->once()
->andReturnUsing(function($file) use ($mock_error_file_path, $mock_error_file_source) {
if ($file === $mock_error_file_path) {
return $mock_error_file_source;
}
return file($file);
});
$notifier->shouldReceive('get_source_file_reader')
->andReturn($reader);
$Exception = new ErrorException('foo', 1, 1, $mock_error_file_path, 5);

// act
$notifier->report_exception($Exception);

// assert
$mock_error_file_frame = null;
foreach($payload['data']['body']['trace']['frames'] as $frame) {
if($frame['filename'] === $mock_error_file_path) {
$mock_error_file_frame = $frame;
break;
}
}
$this->assertNotNull($mock_error_file_frame);
$this->assertEquals('public function getBaz($qux) { return $qux; }', $mock_error_file_frame['code']);
$this->assertEquals(array(
'<?php',
'',
'class Foo extends Bar {',
''
), $mock_error_file_frame['context']['pre']);
$this->assertEquals(array(
'',
'private function getFred() { return 123; }',
'}'
), $mock_error_file_frame['context']['post']);
}

/* --- Internal exceptions --- */

public function testInternalExceptionInReportException() {
Expand Down