[UX] Gracefully handle 423 locked

[PVince81]

Whenever a file is locked on the server, the client might receive the HTTP 423 Locked status code.
It will then show a red warning sign on the system tray icon.

Since this is an expected situation, I suggest to remove the warning sign and keep that failure silent. The client will usually try again later and it should succeed.
The client should still log the lock error in the activity log.

Maybe if the file failed uploading after 5+ attempty, it can then start showing visual warnings to the user ?

In the future, once the server-side transactional locking (aka experimental locking aka high-level file locking) is in place (currently disabled by default), such locking situations might happen more often in high-concurrency setups.

This ticket is more about improving the user experience in case a file is temporarily locked that has hopefully a high chance of succeeding the next time, nothing the user should have to worry about.

What do you think ? @dragotin @guruz @ogoffart

@karlitschek as discussed

[karlitschek]

Great. I had this on my todo list too.
@dragotin what do you think?

[ogoffart]

So if i understand that correctly that would be as simple as putting the error 423 as a SoftError in classifyError in owncloudpropagator_p.h

[MTRichards]

Related to #3490, but not the same as 3490 has server side involvement needed.

[ckamm]

In master "423 Locked" is now a soft error, which means it's not as noticable to the user and does not mark the sync as failed.

To trigger a restart of the sync I'd like to know which operations can lead to 423 @PVince81? Is it only for file uploads?

@dragotin @ogoffart Do you think we should promote such an error to NormalError if it happens too often?

If we want to do that, I suggest we extend the "blacklist" table. It already has the purpose of tracking recurring errors between runs. We currently use it to hide known errors for some time. We'd now also use it to escalate repeated SoftErrors after a while.

[PVince81]

@ckamm it can also happen for other operations, for example if you try moving a folder while one of the files inside is being written by another process (exclusive lock).

[PVince81]

or deleting said folder

[ckamm]

@PVince81 Doh, thanks, that makes sense. :)

[ckamm]

Was backported to 2.0.2

[Dianafg76]

@ckamm I tested this issue
I have one question. Thi is the steps tto reproduce

1. Go to the Desktop client
2. Create a New folder (e.g. ONE)
3. Go to the Folder ONE
4. Upload a file
5. Go to the Server Web with the same user
6. Rename the folder ONE for RENAME ONE, move the folder or delete the folder
7. Go to the Desktop client
8. You can see this error
   
   And in de Desktop you can see this
   

My question is: It the red icon should appear?, because @PVince81 said: Since this is an expected situation, I suggest to remove the warning sign and keep that failure silent.

Thank!!

[Dianafg76]

Desktop v ownCloud-2.0.2.2766-nightly20151005.pkg
Server v {"installed":true,"maintenance":false,"version":"8.1.3.0","versionstring":"8.1.3","edition":"Enterprise"}

[ckamm]

@Dianafg76 If I understand correctly you are renaming a folder on the server while a sync is running? I doubt that will trigger a 423 error on the client. I don't know whether there's a good way to manually trigger a 423 for testing purposes?

[Dianafg76]

@ckamm If I understand correctly you are renaming a folder on the server while a sync is running? Yes, That's right

[Dianafg76]

Log

10-06 09:49:05:050 0x7fd25a54a390 OCC::Folder::slotSyncFinished:   ** error Strings:  ("New Folder/IMG_0007.MOV: Error downloading https://xxx/remote.php/webdav/New Folder/IMG_0007.MOV-chunking-3011215277-19-5 - server replied: Not Found (File with name New Folder could not be located)")
10-06 09:49:05:050 0x7fd25a54a390 OCC::Folder::slotSyncFinished:     * owncloud csync thread finished with error
10-06 09:49:05:050 0x7fd25a54a390 OCC::Folder::slotSyncFinished: the last 1 syncs failed

[Dianafg76]

@ckamm
Steps:

1. On the Desktop, Donwload one file
2. Go to the Server
3. Rename the Folder
4. Go to the Desktop client
5. Delete the same folder
   After the download , on the desktop go to the activity option, and you can see :
   

My question is : Is this correct or should be an error.
@ckamm Can you say what are the steps to reproduce this issue, please?

[dragotin]

Well, what happens here is: The client uploads chunks of a file to a directory which vanishes during the upload. So the upload to the old folder is not longer working - and the error message server replied: Not Found (File with name New Folder could not be located) seems correct.

In the next sync run, this should recover correctly. I do not see a simple solution to this.
@Dianafg76 do you agree?

[dragotin]

And btw, why @Dianafg76 might have seen a 423 Locked error here might be caused by this: During the rename of the target folder, the server might return the 423.

[dragotin]

@Dianafg76 I do not really understand your last comment 😄

[Dianafg76]

@dragotin Sorry
I mean in the next synchronization run, It does it correctly

[PVince81]

@Dianafg76 which specific case are you having trouble with ?

[Dianafg76]

@PVince81 I 've written about two cases and I do not know if they are correct

[PVince81]

In general it is very difficult to reproduce a 423 error, unless you have a slow server/client and a lot of luck.

The best is to edit the server code and add a sleep() in the upload code, just before the final rename:
Before this line https://github.com/owncloud/core/blob/v8.2RC1/lib/private/connector/sabre/file.php#L171 add "sleep(10);".

Then:

1. Create a folder "folder"
2. Wait for sync
3. Upload a small file "test.txt" into "folder" with the web UI webdav (it will spend a long time due to the sleep)
4. During the upload, on the client side (desktop client) rename "folder" to "folder2"

The rename should fail with 423 locked because the uploaded file is about to be put into that folder.

[Dianafg76]

@PVince81 I can't reproduce the issue, if I follow your steps and I have not had much luck .

[PVince81]

sorry, I made a mistake. You need to upload with webdav (cadaver) at step 3 because the "sleep" code path is only for webdav. I just tried it locally and could get the 423 error with sync client 2.0.1.

[Dianafg76]

@PVince81 ahhh, OK. Thanks

[PVince81]

I tried again with the sync client master and the while the 423 error appears in the activity log, it doesn't show a red warning, which is the expected behavior. After the lock was gone, the sync client tried again renaming the folder and succeeded.

[Dianafg76]

@PVince81, @dragotin
I tried again with
Desktop v ownCloud-2.0.2.2772-nightly20151008.pkg
Server v {"installed":true,"maintenance":false,"version":"8.2.0.9","versionstring":"8.2 RC1","edition":""}
I can see on the Desktop a red warning

On Activity option I see

After the lock was gone, the sync client tried again renaming the folder and succeeded.

[PVince81]

I didn't see any red warning in the system tray but didn't check the file overlay view. Is that a fancy addition on MacOS ?

[Dianafg76]

@PVince81 I've seen this on the log

10-08 17:17:10:416 0x7fb803526d20 OCC::ownCloudGui::slotSyncStateChange: Sync state changed for folder  "ownCloud1" :  "Sync Running"
10-08 17:17:10:417 0x7fb803526d20 OCC::PropagateRemoteMove::start: virtual void OCC::PropagateRemoteMove::start() "cadaver2" "cadaver23333"
10-08 17:17:10:417 0x7fb803526d20 OCC::AbstractNetworkJob::start: !!! OCC::MoveJob created for "xxxx" + "/cadaver2"
10-08 17:17:10:556 0x7fb803526d20 OCC::AbstractNetworkJob::slotFinished: void OCC::AbstractNetworkJob::slotFinished() 299 "Error downloading https://xxxx52911/remote.php/webdav/cadaver2 - server replied: Locked" QVariant(int, 423)
10-08 17:17:10:557 0x7fb803526d20 OCC::PropagateRemoteMove::slotMoveJobFinished: void OCC::PropagateRemoteMove::slotMoveJobFinished()  QUrl( "https://xxx:52911/remote.php/webdav/cadaver2" )  FINISHED WITH STATUS 299 "Error downloading https://xxx:52911/remote.php/webdav/cadaver2 - server replied: Locked"
10-08 17:17:10:557 0x7fb803526d20 OCC::SyncEngine::slotItemCompleted: void OCC::SyncEngine::slotItemCompleted(const OCC::SyncFileItem &, const OCC::PropagatorJob &) "cadaver2" INSTRUCTION_RENAME 3 "Error downloading https://xxx/remote.php/webdav/cadaver2 - server replied: Locked"
10-08 17:17:10:559 0x7fb803526d20 OCC::SocketApi::sendMessage: SocketApi:  Sending message:  "STATUS:ERROR:/Users/Diana/newcadaver/cadaver23333"
10-08 17:17:10:560 0x7fb803526d20 OCC::SyncEngine::slotItemCompleted: void OCC::SyncEngine::slotItemCompleted(const OCC::SyncFileItem &, const OCC::PropagatorJob &) "cadaver2" INSTRUCTION_RENAME 3 "Error downloading https://xxxx1/remote.php/webdav/cadaver2 - server replied: Locked"
10-08 17:17:10:561 0x7fb803526d20 OCC::SocketApi::sendMessage: SocketApi:  Sending message:  "STATUS:ERROR:/Users/Diana/newcadaver/cadaver23333"
10-08 17:17:10:566 0x7fb803526d20 OCC::SyncEngine::slotItemCompleted: void OCC::SyncEngine::slotItemCompleted(const OCC::SyncFileItem &, const OCC::PropagatorJob &) "" INSTRUCTION_NONE 0 ""
10-08 17:17:10:567 0x7fb803526d20 OCC::SocketApi::sendMessage: SocketApi:  Sending message:  "STATUS:OK:/Users/Diana/newcadaver/"
10-08 17:17:10:568 0x7fb803526d20 OCC::SyncJournalDb::walCheckpoint: void OCC::SyncJournalDb::walCheckpoint() took 0 msec
10-08 17:17:10:569 0x7fb803526d20 OCC::SyncJournalDb::commitInternal: void OCC::SyncJournalDb::commitInternal(const QString &, bool) Transaction commit  "All Finished." 

[PVince81]

Yes this is expected.

The purpose of this ticket is only to not bother the user with error displays when a locking situation happens. From what I see this is already fixed properly since the system tray doesn't show the error any more, and the error appears in the activity log, which is also expected.

The question now is whether the file list overlay should still show an error for locking issues or whether it should also stay green ?

[Dianafg76]

I don't now
@dragotin, can you help us?

[dragotin]

hmm, probably not an error, but the sync icon should stay.

[ckamm]

I can reproduce this and see the STATUS:ERROR:folder2 that the socket api emits.

[ckamm]

I suspect this is a bug in the Mac overlay icons.

Background:

• When a sync job completes, the socket api currently always sends OK or ERROR. That means currently every file with a SoftError will be set to ERROR state. For the particular test case of renaming "folder" to "folder2" while "folder" is locked, this will result in the STATUS:ERROR:folder2 message.
• However, when the file status for "folder2" is queried, the result will be STATUS:NEW:folder2 because the new folder has never been synced correctly.
• In nautilus, the folder correctly shows the blue sync icon.

We should rethink the status we send to connected clients when a sync item completes. I believe it should be the same status we'll send on subsequent RETRIEVE_FILE_STATUS queries. This is a change I don't want to make in the 2.0 branch.

[ckamm]

@Dianafg76 @PVince81 Thanks for confirming that the error handling works and pointing out the odd behavior with the overlay icon on Mac. I'll close this bug for 2.0.2 and make a new one to fix the overlay icon display for these kinds of errors - that issue applies also to other soft errors.

[PVince81]

Agreed