reactphp / filesystem

Evented filesystem access.
MIT License
135 stars 40 forks source link

Missing error handling (e.g. files that do not exist) #11

Open janhapke opened 8 years ago

janhapke commented 8 years ago

I'm trying to read the contents of a file that does not exist, for example like this:

$filesystem->getContents('does-not-exist.txt')->then(
    function($contents) {
        echo $contents
    },
    function($error) {
        echo 'ERROR: ' . $error;
    }
);

Now, since the file does not exist, I would expect the promise to be rejected and thus the second function to be called (and an error message printed). However, nothing happens.

Quickly browsing through the code, I couldn't see any error handling / rejecting of promises at all. Am I missing something? How should I handle errors when reading files?

WyriHaximus commented 8 years ago

Thank you for reporting this @janhapke, I've just pushed 194f259 adding a check for this specific case. Normally I would suggest checking first before running an operation like this as you can also use the objects to create non-existing files and directories. But this is a special case where the check should have ben handled inside the package. Thanks again for reporting :+1:

andig commented 7 years ago

I would actually expect the same using the long form and it's not happening either:

$filesystem->file('watch.php1')->open('r')->then(function ($stream) {
    $buffer = '';
    $deferred = new \React\Promise\Deferred();

    $stream->on('data', function ($data) use (&$buffer) {
        $buffer .= $data;
        echo "$data\n";
    });

    $stream->on('end', function ($data) use ($stream, $deferred, &$buffer) {
        $stream->close();
        $deferred->resolve($buffer);

        echo 'end';
    });

    return $deferred->promise();
}, function ($stream) {
    echo 'error';
});

Same check needs to go for opening for reading? Funny enough it not only doesn't error the loop also keeps running while it stops when an existing file has been fully read.

Update

If thats not possible (actually even if) it would be nice if exists() were chainable:

$filesystem->file('watch.php')->exists()->then(function($file) {
    $file->open('r')->then(function ($stream) {
      ...