Pass the SAPI file upload errors when throwing an exception #243
Conversation
src/UploadedFile.php
Outdated
| @@ -114,7 +126,9 @@ public function __construct($streamOrFile, $size, $errorStatus, $clientFilename | |||
| public function getStream() | |||
| { | |||
| if ($this->error !== UPLOAD_ERR_OK) { | |||
| throw new RuntimeException('Cannot retrieve stream due to upload error'); | |||
| throw new RuntimeException( | |||
| 'Cannot retrieve stream due to upload error: ' . self::$errorMessages[$this->error] | |||
There was a problem hiding this comment.
Use sprintf to improve readability
src/UploadedFile.php
Outdated
| @@ -147,7 +161,9 @@ public function moveTo($targetPath) | |||
| } | |||
|
|
|||
| if ($this->error !== UPLOAD_ERR_OK) { | |||
| throw new RuntimeException('Cannot retrieve stream due to upload error'); | |||
| throw new RuntimeException( | |||
| 'Cannot retrieve stream due to upload error: ' . self::$errorMessages[$this->error] | |||
There was a problem hiding this comment.
What if $this->error contains unsupported (in errorMessages) value?
There was a problem hiding this comment.
This invariant is already enforced in the constructor
if (! is_int($errorStatus)
|| 0 > $errorStatus
|| 8 < $errorStatus
) {
throw new InvalidArgumentException(
'Invalid error status for UploadedFile; must be an UPLOAD_ERR_* constant'
);
}
src/UploadedFile.php
Outdated
| @@ -16,6 +16,18 @@ | |||
|
|
|||
| class UploadedFile implements UploadedFileInterface | |||
| { | |||
| private static $errorMessages = [ | |||
There was a problem hiding this comment.
This is a good use case because it is immutable data. If it wasn't static it would be duplicated and it would increase the memory footprint of this class by a lot.
There was a problem hiding this comment.
Make this a constant instead; PHP has supported constant array values since 5.6, and they're a better fit here than a static member.
|
Can anyone please merge this? |
src/UploadedFile.php
Outdated
| @@ -16,6 +16,18 @@ | |||
|
|
|||
| class UploadedFile implements UploadedFileInterface | |||
| { | |||
| private static $errorMessages = [ | |||
There was a problem hiding this comment.
Make this a constant instead; PHP has supported constant array values since 5.6, and they're a better fit here than a static member.
src/UploadedFile.php
Outdated
| throw new RuntimeException(sprintf( | ||
| 'Cannot retrieve stream due to upload error: %s', | ||
| self::$errorMessages[$this->error] | ||
| )); |
There was a problem hiding this comment.
This can be tested by passing a non-UPLOAD_ERR_OK value to the $errorStatus constructor argument, and then calling this method.
src/UploadedFile.php
Outdated
| throw new RuntimeException(sprintf( | ||
| 'Cannot retrieve stream due to upload error: %s', | ||
| self::$errorMessages[$this->error] | ||
| )); |
Pass the SAPI file upload errors when throwing an exception
- This will be the last minor release before we target 7.1+.
It most likely will, due to the fact that we now use array constant values.
|
I've taken the liberty of making the requested changes and pushing them to your branch, @Erikvv . |
|
Thanks, @Erikvv |
This will save people some time figuring out why the upload isn't working.
Can't think of any tests.