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.

Sign up for GitHub

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

Retry PUT 409 errors - delete and re-create event #964

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+50 −5
Open

Retry PUT 409 errors - delete and re-create event #964

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8044106
Retry PUT 409 errors - delete and re-create event
telotortium Jan 18, 2022
6139d6d
Don't throw exceptions on unprocessed hrefs in get_multi
telotortium Mar 28, 2022
3e5e438
More hacks to ignore errors from Google Calendar
telotortium Mar 13, 2023
966d711
Also ignore 404 errors during delete for conflicts
telotortium Mar 15, 2023
b21e37e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion vdirsyncer/http.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,12 @@ async def request(

logger.debug(response.status)
logger.debug(response.headers)
logger.debug(response.content)
if (
response.status >= 400
and hasattr(response, "content")
and hasattr(response.content, "_buffer")
):
logger.debug(response.content._buffer)

if response.status == 412:
raise exceptions.PreconditionFailed(response.reason)
Expand Down
47 changes: 44 additions & 3 deletions vdirsyncer/storage/dav.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ async def get_multi(self, hrefs):
else:
rv.append((href, Item(raw), etag))
for href in hrefs_left:
raise exceptions.NotFoundError(href)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_multi receives a set of ids and currently returns items with those ids. In this case, one of them is missing -- but callers of this function will expect it to be returned.

I understand the rationale here, but I've an impression that this change will break things for other places using get_multi.

dav_logger.warning(f"Skipping {href}, not found")

for href, item, etag in rv:
yield href, item, etag
Expand Down Expand Up @@ -593,12 +593,52 @@ async def _put(self, href, item, etag):
async def update(self, href, item, etag):
if etag is None:
raise ValueError("etag must be given and must not be None.")
href, etag = await self._put(self._normalize_href(href), item, etag)
try:
href, etag = await self._put(self._normalize_href(href), item, etag)
except aiohttp.ClientResponseError as e:
if e.status == 409:
dav_logger.debug("Conflict, will delete old event and recreate it.")
try:
await self.delete(self._normalize_href(href), None)
dav_logger.debug("Now trying again")
rv = await self._put(self._normalize_href(href), item, None)
except aiohttp.ClientResponseError as delerr:
dav_logger.debug(f"delerr.status = {delerr.status}")
if delerr.status == 403 or delerr.status == 404:
dav_logger.warning("Old event not found, ignoring")
rv = None, None
else:
raise
elif e.status == 403:
dav_logger.debug("Google Calendar refusing update, ignore")
rv = None, None
else:
raise
return etag

async def upload(self, item: Item):
href = self._get_href(item)
rv = await self._put(href, item, None)
try:
rv = await self._put(href, item, None)
except aiohttp.ClientResponseError as e:
if e.status == 409:
dav_logger.debug("Conflict, will delete old event and recreate it.")
try:
await self.delete(href, None)
dav_logger.debug("Now trying again")
rv = await self._put(href, item, None)
except aiohttp.ClientResponseError as delerr:
dav_logger.debug(f"delerr.status = {delerr.status}")
if delerr.status == 403 or delerr.status == 404:
dav_logger.warning("Old event not found, ignoring")
rv = None, None
else:
raise
elif e.status == 403:
dav_logger.debug("Google Calendar refusing update, ignore")
rv = None, None
else:
raise
return rv

async def delete(self, href, etag):
Expand Down Expand Up @@ -633,6 +673,7 @@ def _parse_prop_responses(self, root, handled_hrefs=None):
props = response.findall("{DAV:}propstat/{DAV:}prop")
if props is None or not len(props):
dav_logger.debug(f"Skipping {href!r}, properties are missing.")
dav_logger.debug(f"Response for {href!r}: {etree.tostring(response)}")
continue
else:
props = _merge_xml(props)
Expand Down
1 change: 0 additions & 1 deletion vdirsyncer/sync/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ async def _run_impl(self, a, b):
)
)
href, etag = await self.dest.storage.upload(self.item)
assert href is not None

self.dest.status.insert_ident(
self.ident, ItemMetadata(href=href, hash=self.item.hash, etag=etag)
Expand Down