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.

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

Fix crash on Document.save() if conflict but not _rev #23

Merged

Conversation

adrienverge
Copy link
Contributor

A common pattern is to try to save a document, and pass if it exists:

try:
    await Document(db, "id").save()
except aiocouch.exception.ConflictError:
    pass

Currently, in case of conflict, aiocouch fails with:

[...]
aiohttp.client_exceptions.ClientResponseError: 409...
During handling of the above exception, another exception occurred:
[...]
KeyError: 'rev'

I propose to set rev = None in error handling, if not available.
Another solution would be to always define Document._data["_rev"], but
this has other implications.

Copy link
Member

@bmario bmario left a comment

Choose a reason for hiding this comment

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

Thanks for pointing this issue out. Just a few small changes.

aiocouch/exception.py Outdated Show resolved Hide resolved
aiocouch/exception.py Outdated Show resolved Hide resolved
tests/test_document.py Outdated Show resolved Hide resolved
tests/test_document.py Outdated Show resolved Hide resolved
@adrienverge adrienverge force-pushed the fix/crash-in-eror-handling-if-rev-missing branch from 05ccdcd to 33cec50 Compare March 30, 2020 09:39
@adrienverge
Copy link
Contributor Author

Thanks for your review, PR updated with:

--- a/aiocouch/exception.py
+++ b/aiocouch/exception.py
@@ -62,11 +62,11 @@ def raise_for_endpoint(endpoint, message, exception, exception_type=None):
 
     message_input = {}
 
-    with suppress(KeyError, AttributeError):
+    with suppress(AttributeError):
         message_input["id"] = endpoint.id
         message_input["endpoint"] = endpoint.endpoint
         message_input["rev"] = endpoint._data.get("_rev")
-    with suppress(KeyError, AttributeError):
+    with suppress(AttributeError):
         message_input["document_id"] = endpoint._document.id
         message_input["document_rev"] = endpoint._document._data.get("_rev")
 
--- a/tests/test_document.py
+++ b/tests/test_document.py
@@ -59,9 +59,9 @@ async def test_conflict(filled_database):
         await doc2.save()
 
 
-async def test_conflict_without_rev(filled_database):
-    doc1 = Document(filled_database, "fou")
-    doc2 = Document(filled_database, "fou")
+async def test_conflict_without_rev(database):
+    doc1 = Document(database, "fou")
+    doc2 = Document(database, "fou")
 
     doc1["blub"] = "new"
     await doc1.save()

A common pattern is to try to save a document, and pass if it exists:

    try:
        await Document(db, "id").save()
    except aiocouch.exception.ConflictError:
        pass

Currently, in case of conflict, aiocouch fails with:

    [...]
    aiohttp.client_exceptions.ClientResponseError: 409...
    During handling of the above exception, another exception occurred:
    [...]
    KeyError: 'rev'

I propose to set `rev = None` in error handling, if not available.
Another solution would be to always define `Document._data["_rev"]`, but
this has other implications.
@adrienverge adrienverge force-pushed the fix/crash-in-eror-handling-if-rev-missing branch from 33cec50 to 545ee4b Compare March 30, 2020 10:39
@bmario bmario merged commit eb7643f into metricq:master Mar 30, 2020
@adrienverge adrienverge deleted the fix/crash-in-eror-handling-if-rev-missing branch March 30, 2020 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants