Skip to content
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.

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

Storage API must work with exceptions, not booleans #13792

Closed
PVince81 opened this issue Jan 30, 2015 · 2 comments
Closed

Storage API must work with exceptions, not booleans #13792

PVince81 opened this issue Jan 30, 2015 · 2 comments

Comments

@PVince81
Copy link
Contributor

PVince81 commented Jan 30, 2015

Currently the Storage API is similar to the one from PHP and simply returns false when an error occurred.
This makes it difficult to distinguish whether "false" means "File not found" or "Storage not available".

We should move to using exceptions in such cases and get rid of booleans.

@Xenopathic @icewind1991 @schiesbn @DeepDiver1975

@RobinMcCorkell
Copy link
Member

I think it comes down to catching 3rd party exceptions then wrapping them into our exceptions. Perhaps some system for a storage to define what exceptions may be thrown from it, and what they should 'map' to? For example, the DAV storage backend would map Sabre\DAV\Exception\NotFound to OCP\Files\NotFoundException. Then, without any extra try catch blocks in the storages themselves, a wrapper or the filesystem itself would catch such errors, then wrap them in our exceptions.

try {
    $storage->doSomething();
} catch (\Exception $e) {
    foreach ($storage->getExceptionMap() as $orig => $wrapper) {
        if (is_instance($e, $orig)) {
            throw new $wrapper(..., $e);
        }
    }
    throw new \OCP\Files\UnknownFailureException(..., $e);
}

Only issue I can see is one of performance...

@PVince81
Copy link
Contributor Author

If we consider that exceptions do not happen often, then performance wouldn't be much of an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants