Do not remove federated share on CURLE errors, provide improved notification in UI and exception handling

[mrow4a]

fixes:

• https://github.com/owncloud/enterprise/issues/4311
• implements "TODO" from https://github.com/owncloud/core/blob/v10.6.0/apps/files_sharing/lib/External/Storage.php#L238

problem:
currently (and without some sophisitcated implementation in the future) there is no clean way of deciding of state of federated share. Currently PROPFIND is send, and if 404 not found is returned, then availability of system by probing e.g. status.php endpoint is performed. If status check succeed, federated share is removed. However, customer with large federation setup reported that shares get falsely removed under certain conditions on the server (e.g. some maintenance).

usecase:

• there are two servers, fed1 and fed2. fed1 gets unavailable, user in fed2 removes federated share that he shared with fed1. fed1 never receives share removal. now fed1 becomes available, provide proper notification and log exception in the logs
• due to some maintenance work, server returns CURLE_COULDNT_CONNECT or CURLE_OPERATION_TIMEDOUT or NotFound. This indicated to fed share storage provider that it should "test remote". However, now for some reason all "status checks like status.php" succeed, and we should let the user in fed1 decide what to do with share, instead of blindly removing

to be done:

• provide most basic notification
• investigate further notifications
• tests

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[jvillafanez]

I'm more inclined to remove the auto-delete feature and let the users decide. As long as the error is properly notified to the users and they can remove the failing remote, it shouldn't be a problem.
There could be additional problems such as a temporary network problem, problems with your ISP, server temporary down... so auto-removing the remote share could be a wrong choice if the admins can recover the connection later.

[mrow4a]

@jvillafanez I also was inclined in this direction until I saw how sync clients is handling this. Not so good -> with auto-delete on auth-error [1] it just removes the share, and uploads its local files to the newly created local folder as replacement, however without auto-delete it goes in some very noisy internal server error loop. on not-found error [2] it tests remote , and only if that fails fed share is removed.

This is why I made sure to do "strict not-found check" to exclude all network related problems. I am however myself puzzled whether not to stop testing remote and in that case (when not found is returned) to just throw error to client and let user decide what to do with that. @VicDeo @jvillafanez thoughs?

[1] auth-error is rather clear signal share got removed, it is that this fed share token can no longer be found.
[2] not-found error to me it is not really clear signal as cannot really think of case when this is thrown

[jvillafanez]

however without auto-delete it goes in some very noisy internal server error loop

I guess that's something we should investigate. Maybe the clients should implement an exponential backoff for the automatic retries, and reset the backoff on manual interaction.

I'm ok with the solution, but it seems we have a technical debt we'll have to clean at some point.

[mrow4a]

@jvillafanez There is now extensive logging, and thus if issue happens again we will have better understanding of what happens. I think what has been done is sufficient.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
No Duplication information

[AlexAndBear]

@mrow4a LGTM, merge?