Skip to content

Convert Dropbox Forbidden exception to StorageNotAvailableException - #25104

Merged
PVince81 merged 1 commit into
masterfrom
dropbox-catchratelimit
Jun 16, 2016
Merged

Convert Dropbox Forbidden exception to StorageNotAvailableException#25104
PVince81 merged 1 commit into
masterfrom
dropbox-catchratelimit

Conversation

@PVince81

Copy link
Copy Markdown
Contributor

Fixes #24739

Basically Dropbox's Forbidden exception is almost always about an error in the request, server error or rate limit error, never something about the files directly. So we can assume that it's about the server not being available temporarily and convert it to StorageNotAvailableException.

This prevents the bad code path that returns false and makes the caller think that the file/folder does not exist any more (false positive).

Please review @owncloud/filesystem @faulix

Would be good to backport this to 9.0 and maybe 8.2 too @DeepDiver1975 @dragotin

@PVince81 PVince81 added this to the 9.1-current milestone Jun 14, 2016
@mention-bot

Copy link
Copy Markdown

By analyzing the blame information on this pull request, we identified @MTGap, @icewind1991, @butonic and @Xenopathic to be potential reviewers

@faulix

faulix commented Jun 15, 2016

Copy link
Copy Markdown

I'm testing it now. For backporting (I use 9.0.2) you need to do the following changes in apps/files_external/lib/dropbox.php:

  • Line 30: namespace OCA\Files_External\Lib\Storage; -> namespace OC\Files\Storage;
  • Line 37: require_once __DIR__ . '/../../../3rdparty/Dropbox/autoload.php'; -> require_once __DIR__ . '/../3rdparty/Dropbox/autoload.php';

I will report if something doesn't work.

@guruz

guruz commented Jun 15, 2016

Copy link
Copy Markdown
Contributor

Code and logic looks fine 👍

I checked the Google Drive storage to see if we need to do the same there. I guess not because it will just rethrow any exception which should be OK then..

@PVince81

Copy link
Copy Markdown
Contributor Author

@icewind1991 second review ?

@icewind1991

Copy link
Copy Markdown
Contributor

👍 looks good

@PVince81

Copy link
Copy Markdown
Contributor Author

stable9: #25148

@lock

lock Bot commented Aug 5, 2019

Copy link
Copy Markdown

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock Bot locked as resolved and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Owncloud deletes in the database all Dropbox files if external_files app is blocked/limited

5 participants