Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). E
- A 2xx upload reply that is not JSON or lacks `image_id` or `image_url` now counts as
delivered, with a warning in the log. Before, the Pi retried the upload and the backend
stored the capture twice.
- A 2xx upload reply with a valid `image_id` but a bad or missing `image_url` now keeps the
backend's id. Before, the Pi fell back to its own capture id and lost the backend's.
- A capture whose requeue after pairing was cut off by a crash is no longer lost when the
camera restarts and drains its queue before the next pairing. The queue now finishes the
requeue instead of deleting the image. A full disk while finishing one such capture no
Expand Down
8 changes: 7 additions & 1 deletion app/backend/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from urllib.parse import quote

import httpx
from pydantic import AnyUrl
from pydantic import AnyUrl, ValidationError

from app.backend.plugin_api import PLUGIN_API_PREFIX
from app.core.runtime_context import get_active_runtime_state
Expand Down Expand Up @@ -161,6 +161,12 @@ async def upload_image(
# queue retry and the backend store a duplicate, so keep what parses and drop the rest.
try:
ack = DeviceImageUploadAck.model_validate(payload)
except ValidationError as exc:
logger.warning("Backend stored the image but its reply was unusable: %r", payload)
# Keep the backend's id when it passed on its own: the local capture id is not the backend's.
id_is_bad = any(err["loc"][:1] == ("image_id",) for err in exc.errors())
image_id = payload.get("image_id") if isinstance(payload, dict) and not id_is_bad else None
return UploadedImageInfo(image_id=image_id, image_url=None)
except (TypeError, ValueError):
logger.warning("Backend stored the image but its reply was unusable: %r", payload)
return UploadedImageInfo(image_id=None, image_url=None)
Expand Down
4 changes: 2 additions & 2 deletions app/pairing/routers/local_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@

router = APIRouter(prefix="/system", tags=["system"])

_API_PORT = 8018
API_PORT = 8018


def get_candidate_urls() -> list[str]:
"""Return all non-loopback IPv4 base URLs for this host."""
return [f"http://{ip}:{_API_PORT}" for ip in find_lan_ips()]
return [f"http://{ip}:{API_PORT}" for ip in find_lan_ips()]


def _get_mdns_name() -> str | None:
Expand Down
4 changes: 2 additions & 2 deletions app/pairing/routers/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from app.core.settings import DEFAULT_PAIRING_BACKEND_URL, settings
from app.core.templates_config import templates
from app.observability.logging import build_log_extra
from app.pairing.routers.local_access import _API_PORT, get_candidate_urls
from app.pairing.routers.local_access import API_PORT, get_candidate_urls
from app.pairing.services.credentials import delete_relay_credentials
from app.pairing.services.service import (
PAIRING_CODE_TTL_SECONDS,
Expand Down Expand Up @@ -87,7 +87,7 @@ async def setup_page(request: Request) -> HTMLResponse:
if operator_authenticated:
candidate_urls = get_candidate_urls()
# Strip scheme and port so the template can compose its own URLs.
lan_ips = [u.removeprefix("http://").removesuffix(f":{_API_PORT}") for u in candidate_urls] or ["<this-ip>"]
lan_ips = [u.removeprefix("http://").removesuffix(f":{API_PORT}") for u in candidate_urls] or ["<this-ip>"]
connection_host = lan_ips[0]
pairing_backend_reachable = mode is ConnectionMode.PAIRED or (
mode is ConnectionMode.PAIRING and await _pairing_backend_reachable()
Expand Down
28 changes: 25 additions & 3 deletions tests/unit/backend/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,18 +246,40 @@ async def test_non_json_2xx_counts_as_delivered(self, caplog: pytest.LogCaptureF
patched_client.return_value.post.assert_awaited_once()

async def test_missing_fields_2xx_counts_as_delivered(self, caplog: pytest.LogCaptureFixture) -> None:
"""A 2xx reply without image_url is delivered once: no exception, so no queue retry."""
response = _fake_response(200, {"image_id": "abc"})
"""A 2xx reply without image_url is delivered once and keeps the backend's valid image_id."""
response = _fake_response(200, {"image_id": SAMPLE_SERVER_IMAGE_ID})
with _patch_async_client(response) as patched_client, caplog.at_level(logging.WARNING):
result = await upload_image(
image_bytes=b"\xff\xd8", filename="t.jpg", capture_metadata={}, upload_metadata={}
)

assert result.image_id is None
assert result.image_id == SAMPLE_SERVER_IMAGE_ID
assert result.image_url is None
assert "unusable" in caplog.text
patched_client.return_value.post.assert_awaited_once()

async def test_invalid_image_url_keeps_valid_image_id(self) -> None:
"""A bad image_url drops only the URL: the backend's real id must not be lost."""
response = _fake_response(200, {"image_id": SAMPLE_SERVER_IMAGE_ID, "image_url": "ftp://bad host"})
with _patch_async_client(response):
result = await upload_image(
image_bytes=b"\xff\xd8", filename="t.jpg", capture_metadata={}, upload_metadata={}
)

assert result.image_id == SAMPLE_SERVER_IMAGE_ID
assert result.image_url is None

async def test_invalid_image_id_is_dropped(self) -> None:
"""An invalid image_id is not passed on, even when the URL is fine."""
response = _fake_response(200, {"image_id": "bad id!", "image_url": BACKEND_IMAGE_URL})
with _patch_async_client(response):
result = await upload_image(
image_bytes=b"\xff\xd8", filename="t.jpg", capture_metadata={}, upload_metadata={}
)

assert result.image_id is None
assert result.image_url is None

async def test_requires_pairing_backend_url(self, monkeypatch: pytest.MonkeyPatch) -> None:
"""Uploading without a configured pairing backend must fail cleanly."""
monkeypatch.setattr(settings, "pairing_backend_url", "")
Expand Down
Loading