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

Sync-Client 1.8.0 broken: Sync error if external folder isn't reachable #3113

Closed
cdamken opened this issue Apr 14, 2015 · 14 comments
Closed
Assignees
Labels
blue-ticket ReadyToTest QA, please validate the fix/enhancement type:bug

Comments

@cdamken
Copy link
Contributor

cdamken commented Apr 14, 2015

The first time the user opens this windows folder (windows network drive plugin) he is asked for credentials.

If a user never entered credentials the folder isn't accessible.

Sync-client 1.7.1 and versions and below didn't care about inaccessible external folders.

Since sync-client 1.8.0 a error message is dropped and the client didn't sync any data.

Relevant logs:

Here the most relevant lines (I think):
Sync-client 1.7.1:

04-14 15:31:13:128 0x912fe3c csync_walker: directory:
ownclouds://SERVER_NAME/remote.php/webdav/Home-Cloud
[file_id=31149627oc6c87be83f3]
04-14 15:31:13:128 0x912fe3c _csync_detect_update: Database entry found,
compare: 0 <-> 1428992044, etag: 54db056f8501d <-> 552cb03ee208c, inode: 0
<-> 101679, size: 0 <-> 0, perms: RM <-> RMCK
04-14 15:31:13:128 0x912fe3c _csync_detect_update: file: Home-Cloud,
instruction: INSTRUCTION_EVAL <<=
04-14 15:31:13:128 0x912fe3c oc_module: opendir method called on
ownclouds://SERVER_NAME/remote.php/webdav/Home-Cloud
04-14 15:31:13:434 0x912fe3c oc_module: Neon error code was 1
04-14 15:31:13:434 0x912fe3c oc_module: Neon error code was 1
04-14 15:31:13:434 0x912fe3c oc_module: WRN: propfind named failed with 1,
request error: 503 Service Unavailable
04-14 15:31:13:434 0x912fe3c oc_module: Errno set to 10014
04-14 15:31:13:434 0x912fe3c csync_ftw: Service was not available!

Sync-client 1.8.0:

04-14 15:23:40:429 0x787186c csync_walker: directory:
ownclouds://SERVER_NAME/remote.php/webdav/Home-Cloud
[file_id=31149627oc6c87be83f3]
04-14 15:23:40:429 0x787186c _csync_detect_update: Database entry found,
compare: 0 <-> 1428992044, etag: 54db056f8501d <-> 552cb03ee208c, inode: 0
<-> 101679, size: 0 <-> 0, perms: RM <-> RMCK
04-14 15:23:40:429 0x787186c _csync_detect_update: file: Home-Cloud,
instruction: INSTRUCTION_EVAL <<=
04-14 15:23:40:429 0x787186c csync_ftw: URI without fuzz for
ownclouds://SERVER_NAME/remote.php/webdav/Home-Cloud is
"Home-Cloud"
04-14 15:23:40:429 0x787186c OCC::DiscoveryJob::remote_vio_opendir_hook:
static csync_vio_handle_t* OCC::DiscoveryJob::remote_vio_opendir_hook(const
char*, void*) OCC::DiscoveryJob(0x7933a98) Home-Cloud Calling into main
thread...
04-14 15:23:40:477 0x5c30ab8 OCC::AbstractNetworkJob::start: !!!
OCC::LsColJob created for QUrl( "https://SERVER_NAME" )
querying "/Home-Cloud"
04-14 15:23:40:742 0x5c30ab8 OCC::AbstractNetworkJob::slotFinished: void
OCC::AbstractNetworkJob::slotFinished() 403 "Error downloading
https://SERVER_NAME/remote.php/webdav/Home-Cloud - server
replied: Service Unavailable"
04-14 15:23:40:742 0x5c30ab8
OCC::DiscoverySingleDirectoryJob::lsJobFinishedWithErrorSlot: void
OCC::DiscoverySingleDirectoryJob::lsJobFinishedWithErrorSlot(QNetworkReply*)
"Error downloading
https://SERVER_NAME/remote.php/webdav/Home-Cloud - server
replied: Service Unavailable" 503 403
04-14 15:23:40:742 0x5c30ab8
OCC::DiscoveryMainThread::singleDirectoryJobFinishedWithErrorSlot: void
OCC::DiscoveryMainThread::singleDirectoryJobFinishedWithErrorSlot(int,
QString) 10014 "Error downloading
https://SERVER_NAME/remote.php/webdav/Home-Cloud - server
replied: Service Unavailable"
04-14 15:23:40:742 0x787186c OCC::DiscoveryJob::remote_vio_opendir_hook:
static csync_vio_handle_t* OCC::DiscoveryJob::remote_vio_opendir_hook(const
char*, void*) OCC::DiscoveryJob(0x7933a98) Home-Cloud ...Returned from main
thread
04-14 15:23:40:742 0x787186c OCC::DiscoveryJob::remote_vio_opendir_hook:
static csync_vio_handle_t* OCC::DiscoveryJob::remote_vio_opendir_hook(const
char*, void*) 10014 when opening Home-Cloud msg= "Error downloading
https://SERVER_NAME/remote.php/webdav/Home-Cloud - server
replied: Service Unavailable"
04-14 15:23:40:742 0x787186c csync_ftw: opendir failed for
ownclouds://SERVER_NAME/remote.php/webdav/Home-Cloud -
errno 10014
04-14 15:23:40:742 0x787186c OCC::DiscoveryJob::remote_vio_closedir_hook:
static void OCC::DiscoveryJob::remote_vio_closedir_hook(csync_vio_handle_t*,
void*) OCC::DiscoveryJob(0x7933a98) ""
04-14 15:23:40:742 0x787186c csync_statedb_close: sqlite3_close=0
04-14 15:23:40:742 0x5c30ab8 OCC::SyncEngine::handleSyncError: #### ERROR
during csync_update : "The service is temporarily unavailable Error
downloading
https://SERVER_NAME/remote.php/webdav/Home-Cloud - server
replied: Service Unavailable"
04-14 15:23:40:742 0x5c30ab8 OCC::SyncEngine::finalize: CSync run took 4583
04-14 15:23:40:758 0x5c30ab8 OCC::Folder::slotSyncFinished: -> SyncEngine
finished without problem.

Full Log files:
S3-ownCloud\Shared\owncloud\support\github-issues\client\3113

@MorrisJobke

00002881

@dragotin
Copy link
Contributor

@guruz 503 for a directory means the directory needs to be ignored.

I suggest this patch to handle this properly:

diff --git a/csync/src/csync_update.c b/csync/src/csync_update.c
index 1c89a87..ddac969 100644
--- a/csync/src/csync_update.c
+++ b/csync/src/csync_update.c
@@ -621,11 +621,11 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn,
           if (asp < 0) {
               CSYNC_LOG(CSYNC_LOG_PRIORITY_ERROR, "asprintf failed!");
           }
-      } else if(errno == ERRNO_STORAGE_UNAVAILABLE) {
-          CSYNC_LOG(CSYNC_LOG_PRIORITY_WARN, "Storage was not available!");
+      } else if(errno == ERRNO_STORAGE_UNAVAILABLE || errno == ERRNO_SERVICE_UNAVAILABLE) {
+          CSYNC_LOG(CSYNC_LOG_PRIORITY_WARN, "Storage or Service was not available!");
           if (ctx->current_fs) {
               ctx->current_fs->instruction = CSYNC_INSTRUCTION_IGNORE;
               ctx->current_fs->error_status = CSYNC_STATUS_STORAGE_UNAVAILABLE;
              /* If a directory has ignored files, put the flag on the parent directory as well */
               if( previous_fs ) {
                   previous_fs->has_ignored_files = true;

Lets discuss tomorrow.

@dragotin dragotin added this to the 1.8.1 - Bugfix milestone Apr 14, 2015
@ogoffart ogoffart assigned guruz and unassigned ogoffart Apr 17, 2015
@ckamm
Copy link
Contributor

ckamm commented Apr 22, 2015

Please see #2884 for why the distinction between 503 Service Unavailable and 503 Storage Unavailable was made.

@moscicki
Copy link
Contributor

@ckamm, @dragotin: I see the distiction between the root folder and other
folders. How does this play in the following cases:

  • multiple sync folders (so not to the root folder)
  • a branded remote home folder which is not a root folder

Thanks for clarification.

kuba

On Wed, Apr 22, 2015 at 11:52 AM, ckamm [email protected] wrote:

Please see #2884 #2884 for why
the distinction between 503 Service Unavailable and 503 Storage Unavailable
was made.


Reply to this email directly or view it on GitHub
#3113 (comment).


Best regards,
Kuba

@ckamm
Copy link
Contributor

ckamm commented Apr 24, 2015

@moscicki I the server will reply "503 Service Unavailable" with the "Sabre\DAV\Exception\ServiceUnavailable" body for any folder in maintenance mode. A "503 Storage Unavailable" also makes no distinctions between root and other folders. Is this what you meant?

@moscicki
Copy link
Contributor

@ckamm: thanks for clarification. I guess you are referring to this line:

if ( body.contains("Sabre\\DAV\\Exception\\ServiceUnavailable") ) {

I understand that the client recognises the server maintenance mode if 503 and the body match the error string. What happens when 503 and the body DOES NOT match the error string --> i.e. "a regular 503 from somewhere else? as per your comment". What happens then? Would you start deleting files?

For the implementation, sorry to say but this is extremely bad design: it actually binds the client to a low-level implementation detail of the server such as the fact of using SabreDav. This does not only break any idea of a network protocol (and protocol encapsulation) but will also break the moment this particular error body changes in the server implementation.

@ckamm
Copy link
Contributor

ckamm commented Apr 24, 2015

@moscicki The maintenance-503 and regular 503 are handled exactly the same during discovery, so that will produce no different behavior. During discovery only the magic '503 Storage not available', will be treated completely differently (by temporarily ignoring the folder).

The specific handling of the server's 503 message only triggers different error reporting to the user. We don't want to bother users about temporary server maintenance. @dragotin agrees that we could extend that to any 503 error and remove the special casing.

I agree that binding to that SabreDav http body and abusing 503 for the "Storage not available" message is brittle and should be avoided. We can skip the former by giving up on distinguishing oc-maintenance-503 and any-other-503, but we're stuck with the latter for now.

@felixboehm
Copy link
Contributor

@dragotin will this be fixed in 1.8.2 ? /cc @MorrisJobke

@danimo
Copy link
Contributor

danimo commented May 26, 2015

@felixboehm:

$ git log origin/1.8|grep "#3113" -B5

commit 73e2254a808c85afa54f2d54497b120b36e122fc
Author: Christian Kamm <[email protected]>
Date:   Fri Apr 24 11:32:47 2015 +0200

    AccountState: Treat *any* 503 as a temporary error. #3113

$ git tag --contains 73e2254a808c85afa54f2d54497b120b36e122fc
v1.8.2-beta1

So yes, it will be in 1.8, and can already be tested by trying out beta1: https://owncloud.org/install/#testing-development

@danimo
Copy link
Contributor

danimo commented May 26, 2015

Btw: this is also visible in the web interface from the commit linked above:

image

@timm2k
Copy link

timm2k commented Jun 1, 2015

For me behavior of 1.8.2beta1 didn't change from 1.8.0 since a WND-mount isn't available through invalid credentials for example. Error message in SyncClient is "Sync Error". No data is synced.

SyncClient before 1.8.0 simply skips those directories and syncs all the rest. This is the behavior we expect.

Regards,
Timm

Log of 1.8.2beta1

06-01 09:33:00:127 0x29ea310 csync_walker: directory: ownclouds://owncloud.example.org/remote.php/webdav/WindowsNetworkDrive [file_id=31149627oc6c87be83f3] 06-01 09:33:00:127 0x29ea310 _csync_detect_update: file: WindowsNetworkDrive, instruction: INSTRUCTION_NEW <<= 06-01 09:33:00:127 0x29ea310 csync_ftw: URI without fuzz for ownclouds://owncloud.example.org/remote.php/webdav/WindowsNetworkDrive is "WindowsNetworkDrive" 06-01 09:33:00:127 0x29ea310 discoveryphase.cpp:426 static csync_vio_handle_t* OCC::DiscoveryJob::remote_vio_opendir_hook(const char*, void*) OCC::DiscoveryJob(0x2c771c0) WindowsNetworkDrive Calling into main thread... 06-01 09:33:00:174 0x2420bb0 networkjobs.cpp:228 !!! OCC::LsColJob created for "https://owncloud.example.org" + "/WindowsNetworkDrive" 06-01 09:33:00:462 0x2420bb0 networkjobs.cpp:161 void OCC::AbstractNetworkJob::slotFinished() 403 "Error downloading https://owncloud.example.org/remote.php/webdav/WindowsNetworkDrive - server replied: Service Unavailable" 06-01 09:33:00:463 0x2420bb0 discoveryphase.cpp:297 void OCC::DiscoverySingleDirectoryJob::lsJobFinishedWithErrorSlot(QNetworkReply*) "Error downloading https://owncloud.example.org/remote.php/webdav/WindowsNetworkDrive - server replied: Service Unavailable" 503 403 06-01 09:33:00:464 0x2420bb0 discoveryphase.cpp:380 void OCC::DiscoveryMainThread::singleDirectoryJobFinishedWithErrorSlot(int, QString) 10014 "Error downloading https://owncloud.example.org/remote.php/webdav/WindowsNetworkDrive - server replied: Service Unavailable" 06-01 09:33:00:466 0x29ea310 discoveryphase.cpp:437 static csync_vio_handle_t* OCC::DiscoveryJob::remote_vio_opendir_hook(const char*, void*) OCC::DiscoveryJob(0x2c771c0) WindowsNetworkDrive ...Returned from main thread 06-01 09:33:00:466 0x29ea310 discoveryphase.cpp:441 static csync_vio_handle_t* OCC::DiscoveryJob::remote_vio_opendir_hook(const char*, void*) 10014 when opening WindowsNetworkDrive msg= "Error downloading https://owncloud.example.org/remote.php/webdav/WindowsNetworkDrive - server replied: Service Unavailable" 06-01 09:33:00:466 0x29ea310 csync_ftw: opendir failed for ownclouds://owncloud.example.org/remote.php/webdav/WindowsNetworkDrive - errno 10014 06-01 09:33:00:466 0x29ea310 discoveryphase.cpp:475 static void OCC::DiscoveryJob::remote_vio_closedir_hook(csync_vio_handle_t*, void*) OCC::DiscoveryJob(0x2c771c0) "" 06-01 09:33:00:466 0x29ea310 csync_statedb_close: sqlite3_close=0 06-01 09:33:00:477 0x2420bb0 syncengine.cpp:538 #### ERROR during csync_update : "Der Dienst ist vorübergehend nicht erreichbar Error downloading https://owncloud.example.org/remote.php/webdav/WindowsNetworkDrive - server replied: Service Unavailable" 06-01 09:33:00:480 0x2420bb0 syncengine.cpp:886 CSync run took 61452 06-01 09:33:00:482 0x2420bb0 folder.cpp:877 -> SyncEngine finished without problem. 06-01 09:33:00:483 0x2420bb0 folder.cpp:416 Processing result list and logging took 0 Milliseconds. 06-01 09:33:00:484 0x2420bb0 folder.cpp:442 OO folder slotSyncFinished: result: 1 06-01 09:33:00:485 0x2420bb0 folder.cpp:902 ** csync not available. 06-01 09:33:00:486 0x2420bb0 folder.cpp:919 the last 4 syncs failed 06-01 09:33:00:487 0x2420bb0 owncloudgui.cpp:257 Folder in overallStatus Message: OCC::Folder(0x26891a0) with name "ownCloud" 06-01 09:33:00:488 0x2420bb0 owncloudgui.cpp:182 Sync state changed for folder "ownCloud" : "Error" 06-01 09:33:00:689 0x2420bb0 folderman.cpp:705 <===================================== sync finished for "ownCloud"

@dragotin
Copy link
Contributor

dragotin commented Jun 3, 2015

@timm2k can you provide what you find in the access_log for the folders in question?

@guruz guruz assigned ckamm and unassigned guruz Jun 3, 2015
@ckamm
Copy link
Contributor

ckamm commented Jun 3, 2015

@danimo @timm2k @dragotin My patch was not intended to fix this issue, it just resolved a side discussion with @moscicki. If we want to ignore folders for which we get 503 (independently of whether it's "service unavailable" or "storage not available") @dragotin's patch from #3113 (comment) looks good.

I wonder why the server doesn't reply "storage not available" in this case...

@ckamm ckamm added the ReadyToTest QA, please validate the fix/enhancement label Jun 4, 2015
@timm2k
Copy link

timm2k commented Jun 9, 2015

1.8.2-final solve my reported issue. Thanks for your work!

@dragotin
Copy link
Contributor

dragotin commented Jun 9, 2015

@timm2k thanks for reporting back, closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blue-ticket ReadyToTest QA, please validate the fix/enhancement type:bug
Projects
None yet
Development

No branches or pull requests