fix: skip download for notes that return server side error

Author: vzhd1701

evernote_backup/note_synchronizer.py

@@ -7,6 +7,7 @@
 from typing import Any, Dict, Iterable, List, Optional, Tuple
 
 from click import progressbar
+from evernote.edam.error.ttypes import EDAMErrorCode, EDAMSystemException
 from evernote.edam.notestore.ttypes import SyncChunk
 from evernote.edam.type.ttypes import LinkedNotebook, Note
 
@@ -33,6 +34,10 @@ class WorkerStopException(Exception):
     """Raise when workers are stopped"""
 
 
+class NoteDownloadException(Exception):
+    """Raise when downloading note fails"""
+
+
 def get_note_size(note: Note) -> int:
     size = note.contentLength
 
@@ -146,13 +151,25 @@ def download_note(self, note_id: str) -> Note:
         for _ in range(retry_count):
             try:
                 return self._note_client.get_note(note_id)
+            except EDAMSystemException as e:
+                if e.errorCode == EDAMErrorCode.RATE_LIMIT_REACHED:
+                    raise
+
+                error_code_name = EDAMErrorCode._VALUES_TO_NAMES.get(
+                    e.errorCode, f"UNKNOWN [{e.errorCode}]"
+                )
+
+                raise NoteDownloadException(
+                    f"Remote server returned system error ({error_code_name} - {e.message})"
+                    f" while downloading note [{note_id}]"
+                )
             except struct.error:
                 logger.debug(
                     f"Remote server returned bad data"
                     f" while downloading note [{note_id}], retrying..."
                 )
 
-        raise RuntimeError(
+        raise NoteDownloadException(
             f"Failed to download note [{note_id}] after {retry_count} attempts!"
         )
 
@@ -379,12 +396,24 @@ def _process_download_chunk(
             for note_f in as_completed(note_futures):
                 f_exc = note_f.exception()
                 if f_exc is not None:
-                    logger.critical(
-                        f"Exception caught while downloading note"
-                        f" '{note_futures[note_f]}'!"
-                    )
-
-                    raise f_exc
+                    if isinstance(f_exc, NoteDownloadException):
+                        logger.error(f_exc)
+                        logger.warning(
+                            f"Note '{note_futures[note_f]}' will be skipped for this run"
+                            " and retried during the next sync."
+                        )
+                        notes_bar.update(1)
+                        continue
+                    elif isinstance(f_exc, EDAMSystemException):
+                        # Only for rate limit error
+                        raise f_exc
+                    else:
+                        logger.critical(
+                            f"Unknown exception caught while downloading note"
+                            f" '{note_futures[note_f]}'!"
+                        )
+
+                        raise f_exc
 
                 note = note_f.result(timeout=120)  # noqa: WPS432
                 self.storage.notes.add_note(note)

tests/test_op_sync.py

@@ -3,6 +3,7 @@
 from hashlib import md5
 
 import pytest
+from evernote.edam.error.ttypes import EDAMErrorCode, EDAMSystemException
 from evernote.edam.type.ttypes import (
     Data,
     LinkedNotebook,
@@ -588,15 +589,15 @@ def fake_slow_get_note(note_guid):
 
 
 @pytest.mark.usefixtures("fake_init_db")
-def test_sync_exception_while_download(
-    cli_invoker, mock_evernote_client, fake_storage, mocker
+def test_sync_unknown_exception_while_download(
+    cli_invoker, mock_evernote_client, fake_storage, mocker, caplog
 ):
-    test_notes = [Note(guid=f"id{i}", title="test") for i in range(100)]
+    test_notes = [Note(guid=f"id{i}", title="test") for i in range(10)]
 
     mock_evernote_client.fake_notes.extend(test_notes)
 
     def fake_get_note(note_guid):
-        if note_guid == "id10":
+        if note_guid == "id3":
             raise RuntimeError("Test error")
 
         return Note(
@@ -613,22 +614,99 @@ def fake_get_note(note_guid):
     )
     mock_get_note.side_effect = fake_get_note
 
+    # with caplog.at_level(logging.DEBUG):
     with pytest.raises(RuntimeError) as excinfo:
         cli_invoker("sync", "--database", "fake_db")
 
     assert str(excinfo.value) == "Test error"
+    assert "Unknown exception caught" in caplog.text
+
+
+@pytest.mark.usefixtures("fake_init_db")
+def test_sync_edam_exception_while_download(
+    cli_invoker, mock_evernote_client, fake_storage, mocker, caplog
+):
+    test_notes = [Note(guid=f"id{i}", title="test") for i in range(10)]
+
+    mock_evernote_client.fake_notes.extend(test_notes)
+
+    def fake_get_note(note_guid):
+        if note_guid == "id3":
+            raise EDAMSystemException(
+                errorCode=EDAMErrorCode.INTERNAL_ERROR, message="Test error"
+            )
+
+        return Note(
+            guid=note_guid,
+            title="test",
+            content="test",
+            notebookGuid="test",
+            contentLength=100,
+            active=True,
+        )
+
+    mock_get_note = mocker.patch(
+        "evernote_backup.evernote_client_sync.EvernoteClientSync.get_note"
+    )
+    mock_get_note.side_effect = fake_get_note
+
+    cli_invoker("sync", "--database", "fake_db")
+
+    assert "INTERNAL_ERROR - Test error" in caplog.text
+    assert (
+        "Note 'test' will be skipped for this run and retried during the next sync"
+        in caplog.text
+    )
+
+
+@pytest.mark.usefixtures("fake_init_db")
+def test_sync_edam_rate_limit_exception_while_download(
+    cli_invoker, mock_evernote_client, fake_storage, mocker, caplog
+):
+    test_notes = [Note(guid=f"id{i}", title="test") for i in range(10)]
+
+    mock_evernote_client.fake_notes.extend(test_notes)
+
+    def fake_get_note(note_guid):
+        if note_guid == "id3":
+            raise EDAMSystemException(
+                errorCode=EDAMErrorCode.RATE_LIMIT_REACHED,
+                message="Test rate limit",
+                rateLimitDuration=10,
+            )
+
+        return Note(
+            guid=note_guid,
+            title="test",
+            content="test",
+            notebookGuid="test",
+            contentLength=100,
+            active=True,
+        )
+
+    mock_get_note = mocker.patch(
+        "evernote_backup.evernote_client_sync.EvernoteClientSync.get_note"
+    )
+    mock_get_note.side_effect = fake_get_note
+
+    with pytest.raises(EDAMSystemException) as excinfo:
+        cli_invoker("sync", "--database", "fake_db")
+
+    assert excinfo.value.message == "Test rate limit"
+    assert excinfo.value.errorCode == EDAMErrorCode.RATE_LIMIT_REACHED
+    assert excinfo.value.rateLimitDuration == 10
 
 
 @pytest.mark.usefixtures("fake_init_db")
 def test_sync_exception_while_download_retry_fail(
-    cli_invoker, mock_evernote_client, fake_storage, mocker
+    cli_invoker, mock_evernote_client, fake_storage, mocker, caplog
 ):
-    test_notes = [Note(guid=f"id{i}", title="test") for i in range(100)]
+    test_notes = [Note(guid=f"id{i}", title="test") for i in range(10)]
 
     mock_evernote_client.fake_notes.extend(test_notes)
 
     def fake_get_note(note_guid):
-        if note_guid == "id10":
+        if note_guid == "id3":
             raise struct.error
 
         return Note(
@@ -645,10 +723,13 @@ def fake_get_note(note_guid):
     )
     mock_get_note.side_effect = fake_get_note
 
-    with pytest.raises(RuntimeError) as excinfo:
-        cli_invoker("sync", "--database", "fake_db")
+    cli_invoker("sync", "--database", "fake_db")
 
-    assert "Failed to download note" in str(excinfo.value)
+    assert "Failed to download note [id3] after 5 attempts" in caplog.text
+    assert (
+        "Note 'test' will be skipped for this run and retried during the next sync"
+        in caplog.text
+    )
 
 
 @pytest.mark.usefixtures("fake_init_db")