Discovery: Allow more HTTP error code to be treated as ignored dir

[ogoffart]

The original code from csync was stopping at any error.
But we have been whitelisting soeme http error code one by one
to ignore the directory instead of aborting the sync.
However, as there are more requests to continue the sync in case
of error, just ignore most HTTP errors

Issue #7586

[mmattel]

Questions:

• What happens in case of 403 (forbidden)
• Which situations can cause a 403?
  Wikipedia: The server understood the request, but is refusing to authorize it.
  eg serverbackend SMB where logged in user has a read only account?
• As you have seen in the referenced log example, the server renturned an empty message but a 504.
  Wouldn´t it be a good idea to add the erroring http response to the error message in case there is no server message at the file in the client?

[ogoffart]

What happens in case of 403 (forbidden)

Same as before: the directory is considered ignored.
It will be shown in the "not synchronized" tab, with the error message pointing out to the error.

Which situations can cause a 403?

There is a comment in the code about it: "403 Forbidden can be sent by the server if the file firewall is active."

As you have seen in the referenced log example, the server renturned an empty message but a 504.
Wouldn´t it be a good idea to add the erroring http response to the error message in case there is no server message at the file in the client?

Right, maybe the error message could be improved. We currently rely on whatever Qt provides us as an error message. I was under the assumption that this should already include the error code, but maybe not.

[mmattel]

There is a comment in the code...

Proposal, as in the code is not that userfriendly, a doc change by adding a section in Using the Synchronization Client describing the not synchronized tab and its impacts/reasons/causes would definitely be a better approach.

[michaelstingl]

a doc change by adding a section in Using the Synchronization Client describing the not synchronized tab and its impacts/reasons/causes would definitely be a better approach.

Yeah, plenty of room for improvement in the desktop docs, but out of scope of the PR. Better create new doc issue for tracking.

[TheOneRing]

Looks good

[mmattel]

Right, maybe the error message could be improved

In the comments above,
...as you have seen in the referenced log example, the server renturned an empty message but a 504. Wouldn´t it be a good idea to add the erroring http response to the error message in case there is no server message at the file in the client?

[sagamusix]

Quick question: Would this PR potentially help with continuing discovery even if discovery of some server folders fails with "Operation canceled"? I set up my ownCloud server from scratch recently and none of my clients can do a full rediscovery anymore because the server times out on random folders (seems like it has an issue keeping up with the high number of requests). I would love to test this this PR but it seems like there have been no daily builds for Windows since 2019-11-09.

[michaelstingl]

@sagamusix what http code do you see in the log when „Operation cancelled „?

[sagamusix]

Which log do you mean exactly? The client sync log isn't very helpful, it's empty:

#=#=#=# Syncrun started 2019-11-22T21:52:11Z
#=#=#=# Syncrun finished 2019-11-22T21:57:41Z (last step: 330215 msec, total: 330215 msec)

Should I run the client with --logfile?

[sagamusix]

Here's the --logfile output for the relevant folder that caused the cancellation of the sync job:

11-23 16:09:32:754 [ info sync.discovery ]:	Processing "Demos/Win32/don't_stop_by_portal_process" | valid: true/true/true | mtime: 1542594887/1553950064/1574186754 | size: 0/4096/0 | etag: "5bf221487483a"//"5dd4305fb6358" | checksum: ""//"" | perm: DNVCKR//DNVCKR | fileid: "02247621oc79c8z0bi2n"//"02247621oc79c8z0bi2n" | inode: 14450/14450/ | type: 2/2/2
11-23 16:09:32:754 [ info sync.discovery ]:	Discovered "Demos/Win32/don't_stop_by_portal_process" INSTRUCTION_UPDATE_METADATA OCC::SyncFileItem::Down 2
11-23 16:12:51:454 [ info sync.discovery ]:	STARTING "Demos/Win32/don't_stop_by_portal_process" OCC::ProcessDirectoryJob::NormalQuery "Demos/Win32/don't_stop_by_portal_process" OCC::ProcessDirectoryJob::NormalQuery
11-23 16:12:51:454 [ info sync.accessmanager ]:	6 "PROPFIND" "https://***/remote.php/dav/files/***/Demos/Win32/don't_stop_by_portal_process" has X-Request-ID "ba3fa9db-b0a4-4e3b-bd43-512f1da2c560"
11-23 16:12:51:454 [ info sync.networkjob ]:	OCC::LsColJob created for "https://***" + "/Dokumente/Demos/Win32/don't_stop_by_portal_process" "OCC::DiscoverySingleDirectoryJob"
11-23 16:17:51:448 [ warning sync.networkjob ]:	Network job timeout QUrl("https://***/remote.php/dav/files/***/Demos/Win32/don't_stop_by_portal_process")
11-23 16:17:51:448 [ warning sync.networkjob ]:	QNetworkReply::OperationCanceledError "Zeitüberschreitung bei der Verbindung" QVariant(Invalid)
11-23 16:17:51:448 [ info sync.networkjob.lscol ]:	LSCOL of QUrl("https://***/remote.php/dav/files/***/Demos/Win32/don't_stop_by_portal_process") FINISHED WITH STATUS "OperationCanceledError Zeitüberschreitung bei der Verbindung"
11-23 16:17:51:448 [ warning sync.discovery ]:	LSCOL job error "Operation abgebrochen" 0 QNetworkReply::OperationCanceledError
11-23 16:17:51:448 [ warning sync.discovery ]:	Server error in directory "Demos/Win32/don't_stop_by_portal_process" 0
11-23 16:17:51:449 [ info sync.engine ]:	Sync run took  509484 ms

Note that hundreds of other folders were scanned in the meantime and finished with status "OK" instead. The sync run itself started roughly half a minute before this folder was discovered.
I guess given the QNetworkReply::OperationCanceledError this PR won't help working around my issue.

EDIT: Switching the server back from HTTP2 to HTTP1.1 solves the issue...

[michaelstingl]

@sagamusix Maybe open a new issue to collect details of your problem, instead of hijacking this merged PR