Skip to content

Commit

Permalink
Merge pull request #76 from modethirteen/include_pre_post_code_context
Browse files Browse the repository at this point in the history 
send pre/post code context for exceptions and errors
  • Loading branch information
@Crisfole
Crisfole committed Apr 22, 2016
2 parents 91ab79e + d4bad68 commit 19a37f4
Show file tree
Hide file tree
Showing 2 changed files with 190 additions and 10 deletions.
78 changes: 69 additions & 9 deletions src/rollbar.php
View file
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 $_php_context = null;
Expand All @@ -138,13 +141,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 @@ -657,24 +664,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 @@ -706,23 +723,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 @@ -996,10 +1021,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
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 @@ -499,7 +510,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

0 comments on commit 19a37f4

Please sign in to comment.