fix: harden Data tab against sibling transform crashes and entity races#68
fix: harden Data tab against sibling transform crashes and entity races#68
Conversation
Three independent failure modes that all surface as "No data available for this component." even when the data endpoint returns items: 1. transformFault now accepts status as either a string or an object with an aggregatedStatus field, accepts code/fault_name as fallbacks for fault_code/description, and tolerates an undefined severity. Without these guards a single malformed fault payload threw inside a sibling transform and rejected the whole Promise.all in prefetchResourceCounts. 2. prefetchResourceCounts now wraps each per-resource transform in its own try/catch via a small safeCount helper. One bad resource can no longer wipe out the others. 3. The EntityDetailPanel resource-counts effect now sets a cancelled flag in its cleanup so a late Promise.all from a previously-selected entity cannot overwrite the new entity's topicsData. Also adds a typed access field to ComponentTopic, plumbed from x-medkit.access. DataPanel uses it to hide the write section for access==='read' and to label the section "Write Value" instead of "Publish Message" for scalar writes. closes #67
There was a problem hiding this comment.
Pull request overview
Hardens the Data tab/resource prefetch path so malformed sibling payloads (notably faults) and fast entity navigation no longer cause the Data tab to appear empty despite /<entity>/data returning items; also adds per-topic access metadata to drive write UI behavior.
Changes:
- Make
transformFaultand related transforms more defensive against payload drift; addComponentTopic.accessderived fromx-medkit.access. - Isolate per-resource count transforms in
prefetchResourceCountsso one bad payload can’t reject the whole counts prefetch. - Prevent stale async results from overwriting new entity state in
EntityDetailPanel; adjustDataPanelwrite/publish UI based onaccess.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/types.ts | Adds typed ComponentTopic.access to represent write capability from vendor extension metadata. |
| src/lib/transforms.ts | Hardens fault transform and plumbs x-medkit.access into ComponentTopic during data transform. |
| src/lib/transforms.test.ts | Adds regression/unit tests for new defensive fault handling and access passthrough behavior. |
| src/lib/store.ts | Wraps individual resource count transforms so failures don’t break sibling counts. |
| src/components/EntityDetailPanel.tsx | Guards against selection-race overwrites by ignoring late async results after entity change. |
| src/components/DataPanel.tsx | Hides write UI for access === 'read' and updates label for scalar writes. |
- Guard `xm?.access` with a `typeof === 'string'` check before lowercasing so non-string vendor payloads no longer throw inside the data transform. - Abort in-flight resource-count and entity-data fetches in the EntityDetailPanel cleanup via AbortController, so rapid entity navigation no longer piles up wasted requests. Threaded `signal` through prefetchResourceCounts to its three underlying GETs. - Replace `as never` casts in transformFault alias tests with `as unknown as RawFaultItem` to keep the cast intent typed.
There was a problem hiding this comment.
Pull request overview
Hardens the Entity Detail “Data” tab against sibling resource transform failures and rapid navigation races, while adding support for a typed per-item access mode to hide/adjust the write UI when appropriate.
Changes:
- Make
transformFaulttolerant of schema drift (status as object, field aliases, undefined severity). - Prevent
prefetchResourceCountsfrom failing the whole counts fetch when one resource transform throws; add AbortSignal support through counts/data fetch to avoid entity-selection races. - Add
ComponentTopic.access(fromx-medkit.access) and use it inDataPanelto hide the write section for read-only items and adjust copy.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/types.ts | Adds optional ComponentTopic.access union type for read/write/readwrite modes. |
| src/lib/transforms.ts | Hardens fault transform and plumbs x-medkit.access into ComponentTopic. |
| src/lib/transforms.test.ts | Adds regression tests for fault schema drift and access parsing. |
| src/lib/store.ts | Threads AbortSignal into prefetchResourceCounts and isolates per-resource transforms with try/catch. |
| src/components/EntityDetailPanel.tsx | Adds AbortController + cancellation guard to prevent stale async results overwriting new entity state. |
| src/components/DataPanel.tsx | Uses topic.access to hide write UI for read-only items and adjusts section label. |
| // Label check takes priority over numeric value; critical is checked first. | ||
| let severity: FaultSeverity = 'info'; | ||
| const severityNum = typeof apiFault.severity === 'number' ? apiFault.severity : 0; | ||
| const label = apiFault.severity_label?.toLowerCase() || ''; |
| const canWrite = isConnected && topic.access !== 'read' && !!(topic.type || topic.type_info || topic.data); | ||
| // Use "Write Value" when the gateway told us this is a writable scalar | ||
| // (access === 'write' / 'readwrite'); fall back to "Publish Message" for | ||
| // streaming topics where the operation really is a publish. | ||
| const writeSectionLabel = | ||
| topic.access === 'write' || topic.access === 'readwrite' ? 'Write Value' : 'Publish Message'; |
| return { | ||
| code: apiFault.fault_code, | ||
| message: apiFault.description, | ||
| code: apiFault.fault_code ?? apiFault.code ?? 'unknown', |
There was a problem hiding this comment.
Major - 'unknown' literal collides across 5 downstream sites (round-2 traced end-to-end).
When both fault_code and code are absent AND entity_id falls back too (transforms.ts:135 source.split('/').pop() || 'unknown'), N malformed faults all get (code='unknown', entity_id='unknown'). Downstream effects:
- Store dedup collapse - store.ts:1912
findIndex((f) => f.code === fault.code && f.entity_id === fault.entity_id)keeps only the FIRST. N-1 faults silently dropped onfault_confirmedevents. - React key duplicates - 3 list sites:
FaultsPanel.tsx:418,FaultsDashboard.tsx:822,FaultsDashboard.tsx:341(FaultGroup). Console throws "Two children with same key 'unknown'", DOM reconciliation diverges. - Expand-state corruption -
FaultsPanel.tsx:272+FaultsDashboard.tsx:400useuseState<Set<string>>keyed byfault.code. Clicking expand on fault Initial project structure #1 (code='unknown') also expands Initial SOVD Web UI Setup #2 and [BUG] Invalid Dockerfile paths. #3 (same key). User cannot control rows independently.
Fix options:
- (a) Synthetic unique ID when both missing:
code: apiFault.fault_code ?? apiFault.code ?? `unknown-${Date.now()}-${Math.random().toString(36).slice(2,7)}`- uniqueness only, not cryptographic. - (b) Leave fallback but change every list key + dedup to
${code}:${entity_id}:${timestamp}or a syntheticfault_uuidfield generated at transform time.
Option (a) is a 1-line change and preserves existing dedup semantics. Option (b) is more invasive but fixes the root (single-field identity is inherently fragile).
Add regression test: transformFaultsResponse called with 3 fault items that each lack code + entity_id → result has 3 distinct .code values.
| // `access` is the explicit per-item write capability; when present it | ||
| // overrides the legacy "any typed topic is publishable" heuristic so a | ||
| // read-only data item never surfaces a write form. | ||
| const canWrite = isConnected && topic.access !== 'read' && !!(topic.type || topic.type_info || topic.data); |
There was a problem hiding this comment.
Major - canWrite falsy-zero bug (round-2 traced end-to-end).
!!(topic.type || topic.type_info || topic.data) evaluates false when topic.data === 0 AND both type/type_info are absent:
!!(undefined || undefined || 0) === !!0 === falseThis is reachable: transforms.ts:345 does hasValue = item.value !== undefined, :352 does data: hasValue ? item.value : null. So literal 0 from the gateway is preserved as topic.data: 0 (correct, since 0 is a legitimate reading). If that response also omits x-medkit.ros2.type / x-medkit.type / x-medkit.type_info (primitive scalars often do), the write form is hidden even when access === 'write' is explicitly set.
Concrete scenario: UDS odometer reading at exactly 0, RPM at idle cutoff, counter reset — anything that momentarily reads 0 and has a minimal payload.
Fix (split presence from truthiness):
const hasTypeHint = !!(topic.type || topic.type_info);
const hasValueHint = topic.data !== null && topic.data !== undefined;
const canWrite = isConnected && topic.access !== 'read' && (hasTypeHint || hasValueHint);Or, since access is now the authoritative signal when present, trust it:
const canWrite = isConnected && (
topic.access === 'write' ||
topic.access === 'readwrite' ||
(topic.access !== 'read' && (!!topic.type || !!topic.type_info))
);Add a test: topic = { access: 'write', data: 0 } (no type) → canWrite === true.
Summary
Fixes #67. Three independent regressions surfaced as the same symptom (Data tab shows "No data available" even when
/<entity>/datareturns items):transformFaultis now defensive: acceptsstatusas string or object (aggregatedStatus), acceptscode/fault_nameas fallbacks forfault_code/description, and tolerates undefinedseverity. Without this any malformed fault payload threw inside a sibling transform and rejected the wholePromise.allinprefetchResourceCounts.prefetchResourceCountswraps each per-resource transform in its owntry/catch. One bad resource can no longer wipe out the others.EntityDetailPanelresource-counts effect sets acancelledflag in cleanup, so a late callback from a previously selected entity cannot overwrite the new entity'stopicsData(caught a real race when navigating fast).accessfield onComponentTopic, plumbed fromx-medkit.access.DataPanelhides the write section foraccess==='read'and labels itWrite Valueinstead ofPublish Messagefor scalar writes.Issue
Type
Testing