Skip to content

Commit

Permalink
Enhancement: S3 StreamWrapper error handling (aws#2372)
Browse files Browse the repository at this point in the history
Improve error message for failed writes due to open file handlers
  • Loading branch information
stobrien89 committed Feb 4, 2022
1 parent 3ed5a5f commit fbb71b7
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 1 deletion.
7 changes: 7 additions & 0 deletions .changes/nextrelease/enhancement-streamwrapper.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"type": "enhancement",
"category": "S3",
"description": "Improved error handling for failed writes and appends on unclosed streams."
}
]
13 changes: 12 additions & 1 deletion src/S3/StreamWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,10 @@ public static function register(

public function stream_close()
{
if ($this->body->getSize() === 0 && !($this->isFlushed)) {
if (!$this->isFlushed
&& empty($this->body->getSize())
&& $this->mode !== 'r'
) {
$this->stream_flush();
}
$this->body = $this->cache = null;
Expand Down Expand Up @@ -163,6 +166,14 @@ public function stream_eof()

public function stream_flush()
{
// Check if stream body size has been
// calculated via a flush or close
if($this->body->getSize() === null && $this->mode !== 'r') {
return $this->triggerError(
"Unable to determine stream size. Did you forget to close or flush the stream?"
);
}

$this->isFlushed = true;
if ($this->mode == 'r') {
return false;
Expand Down
33 changes: 33 additions & 0 deletions tests/S3/StreamWrapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1026,4 +1026,37 @@ public function testStatDataIsClearedOnWriteUsingCustomProtocol()

stream_wrapper_unregister('foo');
}

public function contentProvider()
{
return [
['foo'],
['']
];
}

/**
* @expectedException \PHPUnit\Framework\Error\Warning
* @expectedExceptionMessage Unable to determine stream size. Did you forget to close or flush the stream?
* @dataProvider contentProvider
*/
public function testTriggersErrorOnNoFlushOrClose($content)
{
$stream = $this->getMockBuilder(Psr7\Stream::class)
->disableOriginalConstructor()
->getMock();
$stream->expects($this->any())
->method('getSize')
->willReturn(null);

$this->addMockResults(
$this->client,
[
new Result(['Body' => $stream]),
]
);

$stream = fopen('s3://bucket/key', 'a');
fwrite($stream, $content);
}
}

0 comments on commit fbb71b7

Please sign in to comment.