Jelajahi Sumber

feat(support): record slicer CLI versions; harden sidecar update docs

  Issue #1312 follow-up. Investigation traced the "Name cannot be empty"
  report to a sidecar image pre-dating the /profiles/bundle endpoint
  addition. Two changes so the next occurrence is self-diagnosable from
  the support bundle without a manual curl.

  Backend: new _fetch_slicer_health(url) helper does a 2s GET on /health,
  walks every non-dataPath key under checks looking for a version field
  (the wrapper labels both sidecars as checks.orcaslicer regardless of
  which CLI is bundled). _collect_slicer_api_info now exposes
  bambu_studio_version and orcaslicer_version. Strips trailing slash
  before appending /health to avoid double-slash 404s.

  Docs: bambuddy-wiki/docs/features/slicer-api.md gains a Quick Start
  callout that branch-built sidecars don't auto-update, a corrected
  /health troubleshooting entry (both "unknown" version and "orcaslicer"
  field name on bambu-studio-api are cosmetic wrapper bugs, not stale-
  image indicators), a new "Name cannot be empty" troubleshooting entry,
  and an Updating section that requires --no-cache --pull together
  (BuildKit caches the git context separately from layers, so --no-cache
  alone silently reuses the old checkout).
maziggy 2 minggu lalu
induk
melakukan
52d6ac419a
3 mengubah file dengan 214 tambahan dan 16 penghapusan
  1. 0 0
      CHANGELOG.md
  2. 46 5
      backend/app/api/routes/support.py
  3. 168 11
      backend/tests/unit/test_support_helpers.py

File diff ditekan karena terlalu besar
+ 0 - 0
CHANGELOG.md


+ 46 - 5
backend/app/api/routes/support.py

@@ -646,6 +646,45 @@ async def _check_url_reachable(url: str, timeout: float = 2.0) -> bool | None:
         return False
 
 
+async def _fetch_slicer_health(url: str, timeout: float = 2.0) -> dict | None:
+    """Fetch ``/health`` from a slicer sidecar and extract the CLI version.
+
+    Returns ``None`` when ``url`` is empty (so the caller can distinguish
+    "not configured" from "unreachable"). On any failure to fetch or parse,
+    returns ``{"reachable": False, "version": None}``. The slicer-API wrapper
+    labels both sidecars' CLI under ``checks.orcaslicer`` regardless of which
+    slicer is actually bundled (cosmetic wrapper bug), so we read the version
+    from whichever non-``dataPath`` child key exists rather than hardcoding
+    one. This lets the bundle reviewer answer "is the user running the image
+    they think they are?" without a separate curl round-trip.
+    """
+    if not url or not url.strip():
+        return None
+    health_url = url.rstrip("/") + "/health"
+    try:
+        import httpx
+
+        async with httpx.AsyncClient(timeout=timeout, verify=False) as client:  # noqa: S501 — local sidecars often use self-signed
+            r = await client.get(health_url, follow_redirects=False)
+            if r.status_code != 200:
+                return {"reachable": True, "version": None}
+            try:
+                data = r.json()
+            except Exception:
+                return {"reachable": True, "version": None}
+            checks = data.get("checks") if isinstance(data, dict) else None
+            if not isinstance(checks, dict):
+                return {"reachable": True, "version": None}
+            for key, value in checks.items():
+                if key == "dataPath":
+                    continue
+                if isinstance(value, dict) and "version" in value:
+                    return {"reachable": True, "version": value.get("version")}
+            return {"reachable": True, "version": None}
+    except Exception:
+        return {"reachable": False, "version": None}
+
+
 async def _collect_slicer_api_info() -> dict:
     """Reachability check for configured slicer-API sidecars.
 
@@ -697,12 +736,14 @@ async def _collect_slicer_api_info() -> dict:
         "orcaslicer_url_source": ("db" if oc_db else ("env_or_default" if oc_url else "unset")),
     }
     if info["enabled"]:
-        bs_reach, oc_reach = await asyncio.gather(
-            _check_url_reachable(bs_url),
-            _check_url_reachable(oc_url),
+        bs_health, oc_health = await asyncio.gather(
+            _fetch_slicer_health(bs_url),
+            _fetch_slicer_health(oc_url),
         )
-        info["bambu_studio_reachable"] = bs_reach
-        info["orcaslicer_reachable"] = oc_reach
+        info["bambu_studio_reachable"] = (bs_health or {}).get("reachable") if bs_health is not None else None
+        info["bambu_studio_version"] = (bs_health or {}).get("version") if bs_health is not None else None
+        info["orcaslicer_reachable"] = (oc_health or {}).get("reachable") if oc_health is not None else None
+        info["orcaslicer_version"] = (oc_health or {}).get("version") if oc_health is not None else None
     return info
 
 

+ 168 - 11
backend/tests/unit/test_support_helpers.py

@@ -715,6 +715,156 @@ class TestCheckUrlReachable:
         assert result is False
 
 
+class TestFetchSlicerHealth:
+    """Tests for the slicer-API health probe that extracts the bundled CLI
+    version. Knowing the version in the support bundle lets the reviewer
+    confirm the user is running the image they think they are — exactly the
+    diagnostic that was missing when issue #1312 surfaced."""
+
+    def _mock_httpx(self, status_code: int, body):
+        """Construct a patched httpx.AsyncClient that returns a fixed response."""
+        mock_client_cls = MagicMock()
+        mock_client = AsyncMock()
+        mock_client_cls.return_value.__aenter__.return_value = mock_client
+        mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False)
+        mock_response = MagicMock()
+        mock_response.status_code = status_code
+        if isinstance(body, Exception):
+            mock_response.json.side_effect = body
+        else:
+            mock_response.json.return_value = body
+        mock_client.get = AsyncMock(return_value=mock_response)
+        return mock_client_cls, mock_client
+
+    @pytest.mark.asyncio
+    async def test_empty_url_returns_none(self):
+        from backend.app.api.routes.support import _fetch_slicer_health
+
+        assert await _fetch_slicer_health("") is None
+        assert await _fetch_slicer_health("   ") is None
+
+    @pytest.mark.asyncio
+    async def test_parses_version_from_orcaslicer_field(self):
+        """The default sidecar wrapper labels both orca and bambu CLIs under
+        ``checks.orcaslicer``. The probe must read whichever non-dataPath child
+        carries a ``version`` field instead of hardcoding the field name."""
+        from backend.app.api.routes.support import _fetch_slicer_health
+
+        body = {
+            "status": "healthy",
+            "checks": {
+                "orcaslicer": {"available": True, "version": "2.3.2"},
+                "dataPath": {"accessible": True},
+            },
+        }
+        mock_client_cls, mock_client = self._mock_httpx(200, body)
+        with patch("httpx.AsyncClient", mock_client_cls):
+            result = await _fetch_slicer_health("http://orca:3003")
+
+        assert result == {"reachable": True, "version": "2.3.2"}
+        # And the URL was actually composed as /health.
+        mock_client.get.assert_awaited_once()
+        assert mock_client.get.await_args[0][0] == "http://orca:3003/health"
+
+    @pytest.mark.asyncio
+    async def test_parses_version_when_wrapper_uses_bambustudio_field(self):
+        """Future-proofing: if the wrapper is ever fixed to label the bambu CLI
+        as ``bambustudio``, the probe must still pick up the version. The probe
+        walks every non-dataPath key looking for a ``version`` field rather
+        than hardcoding the slicer name."""
+        from backend.app.api.routes.support import _fetch_slicer_health
+
+        body = {
+            "status": "healthy",
+            "checks": {
+                "bambustudio": {"available": True, "version": "02.06.00.51"},
+                "dataPath": {"accessible": True},
+            },
+        }
+        mock_client_cls, _ = self._mock_httpx(200, body)
+        with patch("httpx.AsyncClient", mock_client_cls):
+            result = await _fetch_slicer_health("http://bs:3001")
+
+        assert result == {"reachable": True, "version": "02.06.00.51"}
+
+    @pytest.mark.asyncio
+    async def test_version_unknown_propagates_as_string(self):
+        """The wrapper emits literal ``"unknown"`` when it can't parse the
+        slicer's --help output. We surface that as-is — it's diagnostic on
+        its own (tells the reviewer the regex didn't match)."""
+        from backend.app.api.routes.support import _fetch_slicer_health
+
+        body = {
+            "status": "healthy",
+            "checks": {
+                "orcaslicer": {"available": True, "version": "unknown"},
+                "dataPath": {"accessible": True},
+            },
+        }
+        mock_client_cls, _ = self._mock_httpx(200, body)
+        with patch("httpx.AsyncClient", mock_client_cls):
+            result = await _fetch_slicer_health("http://bs:3001")
+
+        assert result == {"reachable": True, "version": "unknown"}
+
+    @pytest.mark.asyncio
+    async def test_non_200_status_is_reachable_but_no_version(self):
+        """If the URL responds with a non-200, the host is up but the endpoint
+        isn't the expected one — surface reachable=True so the reviewer can
+        spot misconfiguration without conflating it with a network failure."""
+        from backend.app.api.routes.support import _fetch_slicer_health
+
+        mock_client_cls, _ = self._mock_httpx(404, {})
+        with patch("httpx.AsyncClient", mock_client_cls):
+            result = await _fetch_slicer_health("http://bs:3001")
+
+        assert result == {"reachable": True, "version": None}
+
+    @pytest.mark.asyncio
+    async def test_malformed_json_returns_reachable_no_version(self):
+        from backend.app.api.routes.support import _fetch_slicer_health
+
+        mock_client_cls, _ = self._mock_httpx(200, ValueError("not json"))
+        with patch("httpx.AsyncClient", mock_client_cls):
+            result = await _fetch_slicer_health("http://bs:3001")
+
+        assert result == {"reachable": True, "version": None}
+
+    @pytest.mark.asyncio
+    async def test_missing_checks_block_returns_no_version(self):
+        from backend.app.api.routes.support import _fetch_slicer_health
+
+        mock_client_cls, _ = self._mock_httpx(200, {"status": "healthy"})
+        with patch("httpx.AsyncClient", mock_client_cls):
+            result = await _fetch_slicer_health("http://bs:3001")
+
+        assert result == {"reachable": True, "version": None}
+
+    @pytest.mark.asyncio
+    async def test_connection_error_returns_unreachable(self):
+        from backend.app.api.routes.support import _fetch_slicer_health
+
+        with patch("httpx.AsyncClient") as mock_client_cls:
+            mock_client_cls.return_value.__aenter__.side_effect = ConnectionError("boom")
+
+            result = await _fetch_slicer_health("http://nowhere:9999")
+
+        assert result == {"reachable": False, "version": None}
+
+    @pytest.mark.asyncio
+    async def test_strips_trailing_slash_before_appending_health(self):
+        """Defensive: URLs entered with trailing slashes in Settings should
+        still produce a well-formed /health URL (no double-slash)."""
+        from backend.app.api.routes.support import _fetch_slicer_health
+
+        body = {"status": "healthy", "checks": {"orcaslicer": {"available": True, "version": "2.3.2"}}}
+        mock_client_cls, mock_client = self._mock_httpx(200, body)
+        with patch("httpx.AsyncClient", mock_client_cls):
+            await _fetch_slicer_health("http://bs:3001/")
+
+        assert mock_client.get.await_args[0][0] == "http://bs:3001/health"
+
+
 class TestCollectSlicerApiInfo:
     """Tests for the slicer-API info block (configured URLs + reachability).
 
@@ -744,24 +894,28 @@ class TestCollectSlicerApiInfo:
         session_ctx = self._make_settings_session({"use_slicer_api": "false", "preferred_slicer": "bambu_studio"})
         with (
             patch("backend.app.api.routes.support.async_session", session_ctx),
-            patch("backend.app.api.routes.support._check_url_reachable") as mock_check,
+            patch("backend.app.api.routes.support._fetch_slicer_health") as mock_health,
         ):
             info = await _collect_slicer_api_info()
 
-        mock_check.assert_not_called()
+        mock_health.assert_not_called()
         assert info["enabled"] is False
         assert info["preferred"] == "bambu_studio"
         assert info["bambu_studio_url_set_in_db"] is False
         assert info["orcaslicer_url_set_in_db"] is False
         assert "bambu_studio_reachable" not in info
         assert "orcaslicer_reachable" not in info
+        assert "bambu_studio_version" not in info
+        assert "orcaslicer_version" not in info
 
     @pytest.mark.asyncio
     async def test_enabled_runs_reachability_check_for_both_urls(self):
         from backend.app.api.routes.support import _collect_slicer_api_info
 
-        async def fake_check(url, timeout=2.0):
-            return "orca" in url
+        async def fake_health(url, timeout=2.0):
+            if "orca" in url:
+                return {"reachable": True, "version": "2.3.2"}
+            return {"reachable": False, "version": None}
 
         session_ctx = self._make_settings_session(
             {
@@ -773,7 +927,7 @@ class TestCollectSlicerApiInfo:
         )
         with (
             patch("backend.app.api.routes.support.async_session", session_ctx),
-            patch("backend.app.api.routes.support._check_url_reachable", side_effect=fake_check),
+            patch("backend.app.api.routes.support._fetch_slicer_health", side_effect=fake_health),
         ):
             info = await _collect_slicer_api_info()
 
@@ -784,6 +938,8 @@ class TestCollectSlicerApiInfo:
         assert info["orcaslicer_url_source"] == "db"
         assert info["bambu_studio_reachable"] is False
         assert info["orcaslicer_reachable"] is True
+        assert info["bambu_studio_version"] is None
+        assert info["orcaslicer_version"] == "2.3.2"
 
     @pytest.mark.asyncio
     async def test_env_var_fallback_url_pinged_when_db_setting_empty(self):
@@ -799,16 +955,16 @@ class TestCollectSlicerApiInfo:
 
         seen_urls: list[str] = []
 
-        async def fake_check(url, timeout=2.0):
+        async def fake_health(url, timeout=2.0):
             seen_urls.append(url)
-            return True
+            return {"reachable": True, "version": "02.06.00.51"}
 
         # DB has use_slicer_api=true but NO bambu_studio_api_url row, simulating
         # a user who set the URL via the BAMBU_STUDIO_API_URL env var.
         session_ctx = self._make_settings_session({"use_slicer_api": "true", "preferred_slicer": "bambu_studio"})
         with (
             patch("backend.app.api.routes.support.async_session", session_ctx),
-            patch("backend.app.api.routes.support._check_url_reachable", side_effect=fake_check),
+            patch("backend.app.api.routes.support._fetch_slicer_health", side_effect=fake_health),
             patch("backend.app.api.routes.support.settings") as mock_app_settings,
         ):
             # Pydantic-settings would normally do this for us when reading the
@@ -824,6 +980,7 @@ class TestCollectSlicerApiInfo:
         assert info["bambu_studio_url_set_in_db"] is False
         assert info["bambu_studio_url_source"] == "env_or_default"
         assert info["bambu_studio_reachable"] is True
+        assert info["bambu_studio_version"] == "02.06.00.51"
 
     @pytest.mark.asyncio
     async def test_reachability_uses_unredacted_url(self):
@@ -835,9 +992,9 @@ class TestCollectSlicerApiInfo:
 
         seen_urls: list[str] = []
 
-        async def fake_check(url, timeout=2.0):
+        async def fake_health(url, timeout=2.0):
             seen_urls.append(url)
-            return True
+            return {"reachable": True, "version": "unknown"}
 
         session_ctx = self._make_settings_session(
             {
@@ -848,7 +1005,7 @@ class TestCollectSlicerApiInfo:
         )
         with (
             patch("backend.app.api.routes.support.async_session", session_ctx),
-            patch("backend.app.api.routes.support._check_url_reachable", side_effect=fake_check),
+            patch("backend.app.api.routes.support._fetch_slicer_health", side_effect=fake_health),
         ):
             await _collect_slicer_api_info()
 

Beberapa file tidak ditampilkan karena terlalu banyak file yang berubah dalam diff ini