Skip to content

Commit 029679e

Browse files
authored
fix: Handle registry sync validation errors and surface API error details in UI (#2389)
1 parent d9241ac commit 029679e

14 files changed

+490
-15
lines changed

frontend/src/components/admin/admin-org-registry-table.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
} from "@/components/ui/dropdown-menu"
1616
import { toast } from "@/components/ui/use-toast"
1717
import { useAdminOrgRegistry } from "@/hooks/use-admin"
18+
import { getApiErrorDetail } from "@/lib/errors"
1819
import { getRelativeTime } from "@/lib/event-history"
1920

2021
interface AdminOrgRegistryTableProps {
@@ -40,9 +41,12 @@ export function AdminOrgRegistryTable({
4041
})
4142
} catch (error) {
4243
console.error("Failed to sync repository", error)
44+
const detail =
45+
getApiErrorDetail(error) ??
46+
"Failed to sync repository. Please try again."
4347
toast({
4448
title: "Sync failed",
45-
description: "Failed to sync repository. Please try again.",
49+
description: detail,
4650
variant: "destructive",
4751
})
4852
} finally {

frontend/src/components/admin/platform-registry-repos-table.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
} from "@/components/ui/dropdown-menu"
2020
import { toast } from "@/components/ui/use-toast"
2121
import { useAdminRegistryStatus, useAdminRegistrySync } from "@/hooks/use-admin"
22+
import { getApiErrorDetail } from "@/lib/errors"
2223
import { getRelativeTime } from "@/lib/event-history"
2324

2425
export function PlatformRegistryReposTable() {
@@ -37,9 +38,12 @@ export function PlatformRegistryReposTable() {
3738
refetch()
3839
} catch (error) {
3940
console.error("Failed to sync repository", error)
41+
const detail =
42+
getApiErrorDetail(error) ??
43+
"Failed to sync repository. Please try again."
4044
toast({
4145
title: "Sync failed",
42-
description: "Failed to sync repository. Please try again.",
46+
description: detail,
4347
variant: "destructive",
4448
})
4549
} finally {

frontend/src/lib/errors.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { getApiErrorDetail } from "@/lib/errors"
2+
3+
describe("getApiErrorDetail", () => {
4+
it("returns string detail when present", () => {
5+
const error = Object.assign(new Error("Bad Request"), {
6+
body: { detail: "Registry sync validation failed", message: null },
7+
})
8+
9+
expect(getApiErrorDetail(error)).toBe("Registry sync validation failed")
10+
})
11+
12+
it("prefers the API body message when detail is null", () => {
13+
const error = Object.assign(new Error("Bad Request"), {
14+
body: {
15+
detail: null,
16+
message: "Registry sync validation failed with 2 error(s).",
17+
},
18+
})
19+
20+
expect(getApiErrorDetail(error)).toBe(
21+
"Registry sync validation failed with 2 error(s)."
22+
)
23+
})
24+
25+
it("serializes structured detail before falling back to the Error text", () => {
26+
const error = Object.assign(new Error("Bad Request"), {
27+
body: {
28+
detail: {
29+
action: "tracecat.examples.broken",
30+
reason: "Action not found",
31+
},
32+
message: "Internal Server Error",
33+
},
34+
})
35+
36+
expect(getApiErrorDetail(error)).toBe(
37+
JSON.stringify({
38+
action: "tracecat.examples.broken",
39+
reason: "Action not found",
40+
})
41+
)
42+
})
43+
})

frontend/src/lib/errors.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ import type { ApiError } from "@/client"
22

33
export interface TracecatApiError<T = unknown> extends ApiError {
44
readonly body: {
5-
detail: T
5+
detail?: T
6+
message?: string | null
67
}
78
}
89

@@ -40,3 +41,27 @@ export function isRequestValidationErrorArray(
4041
): obj is RequestValidationError[] {
4142
return Array.isArray(obj) && obj.every((o) => isRequestValidationError(o))
4243
}
44+
45+
export function getApiErrorDetail(error: unknown): string | null {
46+
if (!(error instanceof Error)) {
47+
return null
48+
}
49+
50+
const maybeApiError = error as TracecatApiError<unknown>
51+
const detail = maybeApiError.body?.detail
52+
if (typeof detail === "string") {
53+
return detail
54+
}
55+
if (detail != null) {
56+
try {
57+
return JSON.stringify(detail)
58+
} catch {
59+
return error.message
60+
}
61+
}
62+
const message = maybeApiError.body?.message
63+
if (typeof message === "string" && message.length > 0) {
64+
return message
65+
}
66+
return error.message
67+
}

tests/integration/test_install_and_run_custom_remote_registry_flow.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
from tracecat.settings.schemas import GitSettingsUpdate
3838

3939
GIT_SSH_URL = "git+ssh://git@github.com/TracecatHQ/internal-registry.git"
40+
KNOWN_GOOD_REMOTE_COMMIT_SHA = "e2bfd94c35c93f8052c2b97ff542961596ddd3f8"
4041

4142

4243
@pytest.mark.anyio
@@ -190,6 +191,7 @@ async def test_remote_custom_registry_repo() -> None:
190191
)
191192
sync_response = session.post(
192193
f"{base_url}/registry/repos/{repository_id}/sync",
194+
json={"target_commit_sha": KNOWN_GOOD_REMOTE_COMMIT_SHA},
193195
)
194196
assert sync_response.status_code == 200, f"Sync failed: {sync_response.text}"
195197
sync_data = sync_response.json()

tests/unit/api/test_api_admin_tiers.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from fastapi.testclient import TestClient
1010
from tracecat_ee.admin.tiers import router as tiers_router
1111

12+
from tracecat import config as tracecat_config
1213
from tracecat.auth.types import Role
1314
from tracecat.tiers.schemas import OrganizationTierRead, TierRead
1415

@@ -81,7 +82,7 @@ async def test_list_org_tiers_success(
8182
async def test_create_tier_blocked_without_multi_tenant(
8283
client: TestClient, test_admin_role: Role, monkeypatch: pytest.MonkeyPatch
8384
) -> None:
84-
monkeypatch.setattr(tiers_router.config, "TRACECAT__EE_MULTI_TENANT", False)
85+
monkeypatch.setattr(tracecat_config, "TRACECAT__EE_MULTI_TENANT", False)
8586

8687
response = client.post(
8788
"/admin/tiers",
@@ -96,7 +97,7 @@ async def test_update_org_tier_blocked_without_multi_tenant(
9697
client: TestClient, test_admin_role: Role, monkeypatch: pytest.MonkeyPatch
9798
) -> None:
9899
org_id = uuid.uuid4()
99-
monkeypatch.setattr(tiers_router.config, "TRACECAT__EE_MULTI_TENANT", False)
100+
monkeypatch.setattr(tracecat_config, "TRACECAT__EE_MULTI_TENANT", False)
100101

101102
response = client.patch(
102103
f"/admin/tiers/organizations/{org_id}",

tests/unit/test_registry_sync_base_service.py

Lines changed: 77 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
import pytest
77
from sqlalchemy.ext.asyncio import AsyncSession
8+
from temporalio.client import WorkflowFailureError
9+
from temporalio.exceptions import ApplicationError
810

911
from tracecat import config
1012
from tracecat.auth.types import Role
@@ -18,7 +20,8 @@
1820
from tracecat.registry.repositories.schemas import RegistryRepositoryCreate
1921
from tracecat.registry.repositories.service import RegistryReposService
2022
from tracecat.registry.sync.base_service import ArtifactsBuildResult
21-
from tracecat.registry.sync.service import RegistrySyncService
23+
from tracecat.registry.sync.runner import RegistrySyncValidationError
24+
from tracecat.registry.sync.service import RegistrySyncError, RegistrySyncService
2225
from tracecat.registry.versions.service import RegistryVersionsService
2326

2427

@@ -91,9 +94,15 @@ async def test_sync_creates_collision_version_for_manifest_changes(
9194
mocker.patch(
9295
"tracecat.registry.sync.base_service.fetch_actions_from_subprocess",
9396
side_effect=[
94-
SimpleNamespace(actions=first_actions, commit_sha=None),
95-
SimpleNamespace(actions=second_actions, commit_sha=None),
96-
SimpleNamespace(actions=second_actions, commit_sha=None),
97+
SimpleNamespace(
98+
actions=first_actions, commit_sha=None, validation_errors={}
99+
),
100+
SimpleNamespace(
101+
actions=second_actions, commit_sha=None, validation_errors={}
102+
),
103+
SimpleNamespace(
104+
actions=second_actions, commit_sha=None, validation_errors={}
105+
),
97106
],
98107
)
99108

@@ -138,3 +147,67 @@ async def fake_build_and_upload_artifacts(
138147
versions_service = RegistryVersionsService(session, role)
139148
versions = await versions_service.list_versions(repository_id=repo.id)
140149
assert len(versions) == 2
150+
151+
152+
@pytest.mark.anyio
153+
async def test_sync_via_temporal_matches_validation_application_error_before_cause_walk(
154+
session: AsyncSession,
155+
mock_org_id: uuid.UUID,
156+
mocker,
157+
monkeypatch: pytest.MonkeyPatch,
158+
) -> None:
159+
monkeypatch.setattr(config, "TRACECAT__REGISTRY_SYNC_SANDBOX_ENABLED", True)
160+
161+
session.add(
162+
Organization(
163+
id=mock_org_id,
164+
name="Sync Test Org",
165+
slug=f"sync-test-{mock_org_id.hex[:8]}",
166+
is_active=True,
167+
)
168+
)
169+
await session.flush()
170+
171+
role = Role(
172+
type="service",
173+
user_id=mock_org_id,
174+
organization_id=mock_org_id,
175+
workspace_id=uuid.uuid4(),
176+
service_id="tracecat-runner",
177+
)
178+
179+
repos_service = RegistryReposService(session, role)
180+
repo = await repos_service.create_repository(
181+
RegistryRepositoryCreate(origin=DEFAULT_REGISTRY_ORIGIN)
182+
)
183+
184+
validation_error = RegistrySyncValidationError(
185+
{
186+
"tracecat.examples.broken": [],
187+
}
188+
)
189+
try:
190+
raise ApplicationError(
191+
str(validation_error),
192+
non_retryable=True,
193+
type="RegistrySyncValidationError",
194+
) from validation_error
195+
except ApplicationError as app_error:
196+
workflow_error = WorkflowFailureError(cause=app_error)
197+
198+
mock_client = mocker.Mock()
199+
mock_client.execute_workflow = mocker.AsyncMock(side_effect=workflow_error)
200+
mocker.patch(
201+
"tracecat.dsl.client.get_temporal_client",
202+
mocker.AsyncMock(return_value=mock_client),
203+
)
204+
205+
sync_service = RegistrySyncService(session, role)
206+
207+
with pytest.raises(
208+
RegistrySyncError,
209+
match="RegistrySyncValidationError: Registry sync validation failed",
210+
) as exc_info:
211+
await sync_service.sync_repository_v2(repo, commit=False)
212+
213+
assert "workflow failed" not in str(exc_info.value).lower()

0 commit comments

Comments
 (0)