Skip to content

Simplify and improve error handling #56

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

Merged
cboden merged 3 commits into from
Nov 9, 2016
Merged

Simplify and improve error handling #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
45d4c2d
Simplify and improve error handling
clue Aug 14, 2016
bcdfe75
Buffer requires a valid stream resource to operate on
clue Aug 15, 2016
bfe0855
Work around PHP 5.3 not emitting error events
clue Aug 22, 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
67 changes: 25 additions & 42 deletions src/Buffer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ class Buffer extends EventEmitter implements WritableStreamInterface
private $writable = true;
private $loop;
private $data = '';
private $lastError;

public function __construct($stream, LoopInterface $loop)
{
if (!is_resource($stream) || get_resource_type($stream) !== "stream") {
throw new \InvalidArgumentException('First parameter must be a valid stream resource');
}

$this->stream = $stream;
$this->loop = $loop;
$this->lastErrorFlush();
}

public function isWritable()
Expand Down Expand Up @@ -73,15 +75,15 @@ public function close()

public function handleWrite()
{
if (!is_resource($this->stream)) {
$this->emit('error', array(new \RuntimeException('Tried to write to invalid stream.'), $this));

return;
}

$this->lastErrorFlush();

set_error_handler(array($this, 'errorHandler'));
$error = null;
set_error_handler(function ($errno, $errstr, $errfile, $errline) use (&$error) {
$error = array(
'message' => $errstr,
'number' => $errno,
'file' => $errfile,
'line' => $errline
);
});

$sent = fwrite($this->stream, $this->data);

Expand All @@ -94,23 +96,21 @@ public function handleWrite()
// to keep the stream open for further tries to write.
// Should this turn out to be a permanent error later, it will eventually
// send *nothing* and we can detect this.
if ($sent === 0 && $this->lastError['number'] > 0) {
$this->emit('error', array(
new \ErrorException(
$this->lastError['message'],
if ($sent === 0 || $sent === false) {
if ($error === null) {
$error = new \RuntimeException('Send failed');
} else {
$error = new \ErrorException(
$error['message'],
0,
$this->lastError['number'],
$this->lastError['file'],
$this->lastError['line']
),
$this
));
$error['number'],
$error['file'],
$error['line']
);
}

return;
}
$this->emit('error', array(new \RuntimeException('Unable to write to stream: ' . $error->getMessage(), 0, $error), $this));

if ($sent === 0) {
$this->emit('error', array(new \RuntimeException('Send failed'), $this));
return;
}

Expand All @@ -128,21 +128,4 @@ public function handleWrite()
$this->emit('full-drain', array($this));
}
}

private function errorHandler($errno, $errstr, $errfile, $errline)
{
$this->lastError['number'] = $errno;
$this->lastError['message'] = $errstr;
$this->lastError['file'] = $errfile;
$this->lastError['line'] = $errline;
}

private function lastErrorFlush() {
$this->lastError = array(
'number' => 0,
'message' => '',
'file' => '',
'line' => 0,
);
}
}
28 changes: 23 additions & 5 deletions tests/BufferTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@ public function testConstructor()
$buffer->on('error', $this->expectCallableNever());
}

/**
* @covers React\Stream\Buffer::__construct
* @expectedException InvalidArgumentException
*/
public function testConstructorThrowsIfNotAValidStreamResource()
{
$stream = null;
$loop = $this->createLoopMock();

new Buffer($stream, $loop);
}

/**
* @covers React\Stream\Buffer::write
* @covers React\Stream\Buffer::handleWrite
Expand Down Expand Up @@ -249,11 +261,10 @@ public function testWritingToClosedBufferShouldNotWriteToStream()

/**
* @covers React\Stream\Buffer::handleWrite
* @covers React\Stream\Buffer::errorHandler
*/
public function testError()
public function testErrorWhenStreamResourceIsInvalid()
{
$stream = null;
$stream = fopen('php://temp', 'r+');
$loop = $this->createWriteableLoopMock();

$error = null;
Expand All @@ -263,9 +274,16 @@ public function testError()
$error = $message;
});

// invalidate stream resource
fclose($stream);

$buffer->write('Attempting to write to bad stream');

$this->assertInstanceOf('Exception', $error);
$this->assertSame('Tried to write to invalid stream.', $error->getMessage());

// the error messages differ between PHP versions, let's just check substrings
$this->assertContains('Unable to write to stream: ', $error->getMessage());
$this->assertContains(' not a valid stream resource', $error->getMessage(), '', true);
}

public function testWritingToClosedStream()
Expand All @@ -290,7 +308,7 @@ public function testWritingToClosedStream()
$buffer->write('bar');

$this->assertInstanceOf('Exception', $error);
$this->assertSame('fwrite(): send of 3 bytes failed with errno=32 Broken pipe', $error->getMessage());
$this->assertSame('Unable to write to stream: fwrite(): send of 3 bytes failed with errno=32 Broken pipe', $error->getMessage());
}

private function createWriteableLoopMock()
Expand Down