Repository: bvanelli/actualpy (python)

Issue:
Bank sync fails when an account has sync issues
### Checks

- [X] I have checked that this issue has not already been reported.
- [X] I have confirmed this bug exists on the latest version of actualpy.

### Reproducible example

```python
synchronized_transactions = actual.run_bank_sync()
```


### Log output

```shell
3 validation errors for BankSyncTransactionResponseDTO
data.balances
  Field required [type=missing, input_value={'error_type': 'ACCOUNT_N.../login">SimpleFIN</a>.'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.9/v/missing
data.startingBalance
  Field required [type=missing, input_value={'error_type': 'ACCOUNT_N.../login">SimpleFIN</a>.'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.9/v/missing
data.transactions
  Field required [type=missing, input_value={'error_type': 'ACCOUNT_N.../login">SimpleFIN</a>.'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.9/v/missing
  File "/Users/gmichels/Downloads/actualpy/sync.py", line 18, in <module>
    synchronized_transactions = actual.run_bank_sync()
                                ^^^^^^^^^^^^^^^^^^^^^^
pydantic_core._pydantic_core.ValidationError: 3 validation errors for BankSyncTransactionResponseDTO
data.balances
  Field required [type=missing, input_value={'error_type': 'ACCOUNT_N.../login">SimpleFIN</a>.'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.9/v/missing
data.startingBalance
  Field required [type=missing, input_value={'error_type': 'ACCOUNT_N.../login">SimpleFIN</a>.'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.9/v/missing
data.transactions
  Field required [type=missing, input_value={'error_type': 'ACCOUNT_N.../login">SimpleFIN</a>.'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.9/v/missing
```
```


### Issue description

The `run_bank_sync()` operation fails when one of the accounts has known sync issues within SimpleFIN.

### Expected behavior

I'd expect the operation to skip the account that has sync issues and complete successfully for the remaining accounts that are operational (like when running a bank sync from the UI).

### Installed versions

- actualpy version: 0.4.0
- Actual Server version: edge


Pull request:
fix: Handle account sync errors when account is not properly setup.
Test it  locally with the Simplefin demo. Here is what happens when the bank sync fails:

```
Traceback (most recent call last):
  File "/Users/brunno.vanelli/Documents/git/actualpy/dev/bank_sync.py", line 4, in <module>
    added = actual.run_bank_sync()
  File "/Users/brunno.vanelli/Documents/git/actualpy/actual/__init__.py", line 490, in run_bank_sync
    transactions = self._run_bank_sync_account(acct, default_start_date)
  File "/Users/brunno.vanelli/Documents/git/actualpy/actual/__init__.py", line 434, in _run_bank_sync_account
    raise ActualBankSyncError(
actual.exceptions.ActualBankSyncError: ('SERVER_DOWN', 'rejected', 'There was an error communciating with SimpleFIN.')
```

Closes #71 

Patch:
diff --git a/actual/__init__.py b/actual/__init__.py
index 1f2206c..bdf1ed7 100644
--- a/actual/__init__.py
+++ b/actual/__init__.py
@@ -17,7 +17,7 @@ from sqlalchemy import insert, update
 from sqlmodel import MetaData, Session, create_engine, select
 
 from actual.api import ActualServer
-from actual.api.models import RemoteFileListDTO
+from actual.api.models import BankSyncErrorDTO, RemoteFileListDTO
 from actual.crypto import create_key_buffer, decrypt_from_meta, encrypt, make_salt
 from actual.database import (
     Accounts,
@@ -28,7 +28,12 @@ from actual.database import (
     reflect_model,
     strong_reference_session,
 )
-from actual.exceptions import ActualError, InvalidZipFile, UnknownFileId
+from actual.exceptions import (
+    ActualBankSyncError,
+    ActualError,
+    InvalidZipFile,
+    UnknownFileId,
+)
 from actual.migrations import js_migration_statements
 from actual.protobuf_models import HULC_Client, Message, SyncRequest
 from actual.queries import (
@@ -425,6 +430,12 @@ class Actual(ActualServer):
         new_transactions_data = self.bank_sync_transactions(
             sync_method.lower(), account_id, start_date, requisition_id=requisition_id
         )
+        if isinstance(new_transactions_data, BankSyncErrorDTO):
+            raise ActualBankSyncError(
+                new_transactions_data.data.error_type,
+                new_transactions_data.data.status,
+                new_transactions_data.data.reason,
+            )
         new_transactions = new_transactions_data.data.transactions.all
         imported_transactions = []
         for transaction in new_transactions:
diff --git a/actual/api/__init__.py b/actual/api/__init__.py
index 790974d..4c597a3 100644
--- a/actual/api/__init__.py
+++ b/actual/api/__init__.py
@@ -8,8 +8,8 @@ import requests
 
 from actual.api.models import (
     BankSyncAccountResponseDTO,
+    BankSyncResponseDTO,
     BankSyncStatusDTO,
-    BankSyncTransactionResponseDTO,
     BootstrapInfoDTO,
     Endpoints,
     GetUserFileInfoDTO,
@@ -284,7 +284,7 @@ class ActualServer:
         account_id: str,
         start_date: datetime.date,
         requisition_id: str = None,
-    ) -> BankSyncTransactionResponseDTO:
+    ) -> BankSyncResponseDTO:
         if bank_sync == "gocardless" and requisition_id is None:
             raise ActualInvalidOperationError("Retrieving transactions with goCardless requires `requisition_id`")
         endpoint = Endpoints.BANK_SYNC_TRANSACTIONS.value.format(bank_sync=bank_sync)
@@ -292,4 +292,4 @@ class ActualServer:
         if requisition_id:
             payload["requisitionId"] = requisition_id
         response = requests.post(f"{self.api_url}/{endpoint}", headers=self.headers(), json=payload, verify=self.cert)
-        return BankSyncTransactionResponseDTO.model_validate(response.json())
+        return BankSyncResponseDTO.validate_python(response.json())
diff --git a/actual/api/bank_sync.py b/actual/api/bank_sync.py
index c8dd600..c281655 100644
--- a/actual/api/bank_sync.py
+++ b/actual/api/bank_sync.py
@@ -117,3 +117,10 @@ class BankSyncTransactionData(BaseModel):
     # goCardless specific
     iban: Optional[str] = None
     institution_id: Optional[str] = Field(None, alias="institutionId")
+
+
+class BankSyncErrorData(BaseModel):
+    error_type: str
+    error_code: str
+    status: Optional[str] = None
+    reason: Optional[str] = None
diff --git a/actual/api/models.py b/actual/api/models.py
index d7d4227..8be9ebd 100644
--- a/actual/api/models.py
+++ b/actual/api/models.py
@@ -1,11 +1,15 @@
 from __future__ import annotations
 
 import enum
-from typing import List, Optional
+from typing import List, Optional, Union
 
-from pydantic import BaseModel, Field
+from pydantic import BaseModel, Field, TypeAdapter
 
-from actual.api.bank_sync import BankSyncAccountData, BankSyncTransactionData
+from actual.api.bank_sync import (
+    BankSyncAccountData,
+    BankSyncErrorData,
+    BankSyncTransactionData,
+)
 
 
 class Endpoints(enum.Enum):
@@ -158,3 +162,10 @@ class BankSyncAccountResponseDTO(StatusDTO):
 
 class BankSyncTransactionResponseDTO(StatusDTO):
     data: BankSyncTransactionData
+
+
+class BankSyncErrorDTO(StatusDTO):
+    data: BankSyncErrorData
+
+
+BankSyncResponseDTO = TypeAdapter(Union[BankSyncErrorDTO, BankSyncTransactionResponseDTO])
diff --git a/actual/exceptions.py b/actual/exceptions.py
index 55d5aa8..fbea0ea 100644
--- a/actual/exceptions.py
+++ b/actual/exceptions.py
@@ -50,3 +50,8 @@ class ActualDecryptionError(ActualError):
 
 class ActualSplitTransactionError(ActualError):
     pass
+
+
+class ActualBankSyncError(ActualError):
+    def __init__(self, error_type: str, status: str = None, reason: str = None):
+        self.error_type, self.status, self.reason = error_type, status, reason


Test patch:
diff --git a/tests/test_bank_sync.py b/tests/test_bank_sync.py
index f7dd24f..0e9424f 100644
--- a/tests/test_bank_sync.py
+++ b/tests/test_bank_sync.py
@@ -4,7 +4,7 @@ import decimal
 
 import pytest
 
-from actual import Actual
+from actual import Actual, ActualBankSyncError
 from actual.database import Banks
 from actual.queries import create_account
 from tests.conftest import RequestsMock
@@ -54,6 +54,12 @@ response = {
     },
 }
 
+fail_response = {
+    "error_type": "ACCOUNT_NEEDS_ATTENTION",
+    "error_code": "ACCOUNT_NEEDS_ATTENTION",
+    "reason": "The account needs your attention.",
+}
+
 
 def create_accounts(session, protocol: str):
     bank = create_account(session, "Bank")
@@ -140,3 +146,19 @@ def test_bank_sync_unconfigured(mocker, session):
         actual._session = session
         create_accounts(session, "simplefin")
         assert actual.run_bank_sync() == []
+
+
+def test_bank_sync_exception(session, mocker):
+    mocker.patch("requests.get").return_value = RequestsMock({"status": "ok", "data": {"validated": True}})
+    main_mock = mocker.patch("requests.post")
+    main_mock.side_effect = [
+        RequestsMock({"status": "ok", "data": {"configured": True}}),
+        RequestsMock({"status": "ok", "data": fail_response}),
+    ]
+    with Actual(token="foo") as actual:
+        actual._session = session
+        create_accounts(session, "simplefin")
+
+        # now try to run the bank sync
+        with pytest.raises(ActualBankSyncError):
+            actual.run_bank_sync()