-
-
Notifications
You must be signed in to change notification settings - Fork 88
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
Contact sync abort on single vcard error #1351
Comments
I have tried reproducing, and for me it is working as expected. Steps:
And everything is working well. What is your issue? Does the PR mentioned have to be merged on Nextcloud before so that this error occurs? We cannot implement "fixes" for things that still do not happen. Nextcloud could simply never merge the PR, and nothing would have been updated. Also, without reproduction steps, we cannot verify whether this is an issue or not. |
Sorry if i didn't make the point clear. Yes for my reproduction steps, it's needed to merge my mentioned nextcloud PR. But the Problem I see doesn't need nextcloud by itself, just a example server. My Problem is a unclear error handling, if (any Server) would reject a new/updated CardDav (theoretically the same for CalDav) all sync for that server is aborted. Instead I think, when it updates to server, and the server returns a 4xx, it should just abort that one Contact/Calender. # Current: DavX⁵ Updates a VCard ----> PUT to DavServer ---> DavServer Returns error 4xx ----> DavX⁵ stops ALL syncronisation Expected: DavX⁵ Updates a VCard ----> PUT to DavServer ---> DavServer Returns error 4xx ----> DavX⁵ skips this VCard, continues with next vcard |
To understand that correctly: what is an invalid contact / invalid email? Invalid in which sense? For CardDAV, it would be relevant whether the contact / email is a valid vCard. The vCard generated by DAVx5 should never contain syntactically invalid fields (like emails) that make the vCard unparseable. If it's about server-side data validation: As far as I know, WebDAV/CardDAV doesn't specify this behavior and how it should be done. So I have to investigate further. However clients (also other clients) can't be expected to react in a specific way. However the correct usage of status codes may make things more easy.
It depends on the actual status code. 400 is a Bad Request. It leaves the synchronization in an undefined state, as DAVx5 doesn't know what happened. I don't think 400 is the correct status code, because 400 means that the message body is syntactically incorrect [https://www.rfc-editor.org/rfc/rfc4918#section-11.2].
But that would mean that either
What do you think? |
Invalid to the EMail RFC, like spaces at start/end, no @, unable to send messages to.... I know the whole CardDav Scheme has no such validation for any fields.
Ok I'll discuss it again.
I'd always still show an error, the question is just how, currently the user just sees an error (yes on detailed inspection they can see the corresponding VCard, but at first it just looks like a broken server, which is wrong on 4XX). I already realize it's not as straightforward as I thought, considering the RFCs etc. Weird that no (common) system has something similar implemented. But I think it would be a helpful somewhat important feature to have (and a starting point for other to continue). |
Problem scope
App version
Android version and device/firmware type
Android 15 (Google Pixel 6a)
Steps to reproduce
1.5. Add a valid new contact
Actual result
Currently, when a put/post request fails, the entire sync process is aborted, and changes to other contacts (different http-requests) are not synced.
Expected result
I would expect DAVx⁵ to skip the invalid contact and continue syncing the remaining valid contacts instead of aborting the entire sync process.
Maybe: Extract the error message and show it directly in the notification
Further info
The text was updated successfully, but these errors were encountered: