mirror of
https://github.com/vxcontrol/pentagi.git
synced 2026-08-26 13:06:32 +00:00
fix(flows): return partial container listings instead of failing; stop the refetch skeleton flash
Container-listing polish on top of the partial-success work: - A directory whose entries all fail to stat no longer 500s. The find exec already proved the container alive, so ListContainerDir returns the readable entries (possibly none) plus the per-entry failures, and the handler serves a 200 partial listing; only a cancelled request or a dir that can't be listed at all still errors. - Over the entry cap, list the first page and set a Truncated flag rather than erroring with end-user copy from the docker layer. The flag flows through to the Pull dialog, which now warns the user the directory was truncated instead of silently showing a subset. - The Pull dialog no longer flashes a skeleton on an in-place refetch (Refresh, or after a Pull): the listing hook clears its rows only when the path actually changes, so a same-path reload keeps the current rows under the loading guard. Tests: an all-entries-failed listing returns 200 with the failures; the Truncated flag surfaces in the response. Swagger regenerated for the new field. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
2c524faea9
commit
7b67b1c0ea
@@ -597,6 +597,9 @@ type ContainerEntryError struct {
|
||||
type ContainerDirListing struct {
|
||||
Files []container.PathStat
|
||||
Failures []ContainerEntryError
|
||||
// Truncated is set when the directory held more than maxListEntries children
|
||||
// and only the first maxListEntries were listed.
|
||||
Truncated bool
|
||||
}
|
||||
|
||||
func (dc *dockerClient) ListContainerDir(
|
||||
@@ -660,25 +663,27 @@ func (dc *dockerClient) ListContainerDir(
|
||||
entryPaths = append(entryPaths, tok)
|
||||
}
|
||||
|
||||
// Over the cap, list the first maxListEntries and flag the truncation rather
|
||||
// than failing the whole listing — the caller reports the partiality.
|
||||
truncated := false
|
||||
if len(entryPaths) > maxListEntries {
|
||||
return ContainerDirListing{}, fmt.Errorf("container directory '%s' has too many entries (%d, limit %d) — narrow the path", dirPath, len(entryPaths), maxListEntries)
|
||||
entryPaths = entryPaths[:maxListEntries]
|
||||
truncated = true
|
||||
}
|
||||
|
||||
stats, failures := statContainerEntries(ctx, entryPaths, containerListWorkers, func(ctx context.Context, entryPath string) (container.PathStat, error) {
|
||||
return dc.ContainerStatPath(ctx, containerID, entryPath)
|
||||
})
|
||||
|
||||
// A cancelled context, or a listing where every entry failed, is a directory-
|
||||
// level fault (client gone, or the container/daemon died mid fan-out) — not a
|
||||
// partial listing. Surface it as an error rather than a misleading empty 200.
|
||||
// A cancelled context is a directory-level fault (the client is gone), not a
|
||||
// partial listing, so surface it as an error. Entries that individually failed
|
||||
// to stat are carried in Failures — the find exec already proved the container
|
||||
// alive, so a live directory degrades rather than 500s even if every entry failed.
|
||||
if err := ctx.Err(); err != nil {
|
||||
return ContainerDirListing{}, fmt.Errorf("listing container directory '%s': %w", dirPath, err)
|
||||
}
|
||||
if len(entryPaths) > 0 && len(stats) == 0 {
|
||||
return ContainerDirListing{}, fmt.Errorf("failed to read any of the %d entries in container directory '%s' (container may be gone): %w", len(entryPaths), dirPath, failures[0].err)
|
||||
}
|
||||
|
||||
listing := ContainerDirListing{Files: stats}
|
||||
listing := ContainerDirListing{Files: stats, Truncated: truncated}
|
||||
for _, f := range failures {
|
||||
listing.Failures = append(listing.Failures, ContainerEntryError{
|
||||
Name: path.Base(f.name),
|
||||
|
||||
@@ -8374,6 +8374,10 @@ const docTemplate = `{
|
||||
},
|
||||
"total": {
|
||||
"type": "integer"
|
||||
},
|
||||
"truncated": {
|
||||
"description": "Truncated is true when the directory held more entries than the listing cap\nand only the first page was returned.",
|
||||
"type": "boolean"
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
@@ -8366,6 +8366,10 @@
|
||||
},
|
||||
"total": {
|
||||
"type": "integer"
|
||||
},
|
||||
"truncated": {
|
||||
"description": "Truncated is true when the directory held more entries than the listing cap\nand only the first page was returned.",
|
||||
"type": "boolean"
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
@@ -465,6 +465,11 @@ definitions:
|
||||
type: string
|
||||
total:
|
||||
type: integer
|
||||
truncated:
|
||||
description: |-
|
||||
Truncated is true when the directory held more entries than the listing cap
|
||||
and only the first page was returned.
|
||||
type: boolean
|
||||
type: object
|
||||
models.CopyResourceRequest:
|
||||
properties:
|
||||
|
||||
@@ -43,6 +43,9 @@ type ContainerFiles struct {
|
||||
Files []ContainerFile `json:"files"`
|
||||
Failures []ContainerFileError `json:"failures,omitempty"`
|
||||
Total uint64 `json:"total"`
|
||||
// Truncated is true when the directory held more entries than the listing cap
|
||||
// and only the first page was returned.
|
||||
Truncated bool `json:"truncated,omitempty"`
|
||||
}
|
||||
|
||||
// PullFlowFilesRequest is the request body for pulling files from a container.
|
||||
|
||||
@@ -987,6 +987,7 @@ func (s *FlowFileService) GetFlowContainerFiles(c *gin.Context) {
|
||||
// EVERY path fails at the directory level, nothing could be listed and the
|
||||
// request is failed as a whole (checked after the loop).
|
||||
pathsListed := 0
|
||||
truncated := false
|
||||
var firstPathErr error
|
||||
recordPathFailure := func(p string, err error) {
|
||||
if firstPathErr == nil {
|
||||
@@ -1035,6 +1036,9 @@ func (s *FlowFileService) GetFlowContainerFiles(c *gin.Context) {
|
||||
continue
|
||||
}
|
||||
pathsListed++
|
||||
if listing.Truncated {
|
||||
truncated = true
|
||||
}
|
||||
for _, stat := range listing.Files {
|
||||
file := convertContainerFile(containerPath, stat)
|
||||
if _, seen := seenPaths[file.Path]; !seen {
|
||||
@@ -1128,10 +1132,11 @@ func (s *FlowFileService) GetFlowContainerFiles(c *gin.Context) {
|
||||
}
|
||||
|
||||
response.Success(c, http.StatusOK, models.ContainerFiles{
|
||||
Path: responsePath,
|
||||
Files: allFiles,
|
||||
Failures: allFailures,
|
||||
Total: uint64(len(allFiles)),
|
||||
Path: responsePath,
|
||||
Files: allFiles,
|
||||
Failures: allFailures,
|
||||
Total: uint64(len(allFiles)),
|
||||
Truncated: truncated,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -102,3 +102,33 @@ func TestGetFlowContainerFiles_PathNeverInBothArrays(t *testing.T) {
|
||||
"order %q: /work/x must NOT also be in Failures", order)
|
||||
}
|
||||
}
|
||||
|
||||
// A live container whose entries all fail to stat degrades to a 200 partial
|
||||
// listing (empty files, entries in Failures), not a 500.
|
||||
func TestGetFlowContainerFiles_AllEntriesFailedStillReturns200(t *testing.T) {
|
||||
fake := &fakeDockerClient{running: true}
|
||||
fake.statPathMap = map[string]container.PathStat{"/work": {Mode: os.ModeDir | 0755}}
|
||||
fake.listDirMap = map[string][]container.PathStat{"/work": {}}
|
||||
fake.listDirFailMap = map[string][]docker.ContainerEntryError{
|
||||
"/work": {
|
||||
{Name: "a", Path: "/work/a", Err: fmt.Errorf("stat: gone")},
|
||||
{Name: "b", Path: "/work/b", Err: fmt.Errorf("stat: gone")},
|
||||
},
|
||||
}
|
||||
code, resp := listContainerFiles(t, fake, "paths[]=/work")
|
||||
require.Equal(t, http.StatusOK, *code)
|
||||
assert.Empty(t, resp.Files)
|
||||
assert.Len(t, resp.Failures, 2)
|
||||
}
|
||||
|
||||
// A truncated listing surfaces Truncated so the UI can warn the user they are not
|
||||
// seeing every entry.
|
||||
func TestGetFlowContainerFiles_TruncatedFlagSurfaced(t *testing.T) {
|
||||
fake := &fakeDockerClient{running: true}
|
||||
fake.statPathMap = map[string]container.PathStat{"/big": {Mode: os.ModeDir | 0755}}
|
||||
fake.listDirMap = map[string][]container.PathStat{"/big": {{Name: "f", Mode: 0644, Size: 1}}}
|
||||
fake.listDirTruncated = map[string]bool{"/big": true}
|
||||
code, resp := listContainerFiles(t, fake, "paths[]=/big")
|
||||
require.Equal(t, http.StatusOK, *code)
|
||||
assert.True(t, resp.Truncated)
|
||||
}
|
||||
|
||||
@@ -968,11 +968,12 @@ type fakeDockerClient struct {
|
||||
|
||||
// Per-path overrides for multi-path tests; take precedence over the
|
||||
// single-value fields above when the queried path has an entry here.
|
||||
statPathMap map[string]container.PathStat
|
||||
statPathErrMap map[string]error
|
||||
listDirMap map[string][]container.PathStat
|
||||
listDirFailMap map[string][]docker.ContainerEntryError
|
||||
listDirErrMap map[string]error
|
||||
statPathMap map[string]container.PathStat
|
||||
statPathErrMap map[string]error
|
||||
listDirMap map[string][]container.PathStat
|
||||
listDirFailMap map[string][]docker.ContainerEntryError
|
||||
listDirErrMap map[string]error
|
||||
listDirTruncated map[string]bool
|
||||
|
||||
// CopyFromContainer behaviour.
|
||||
copyFromBody []byte
|
||||
@@ -1058,12 +1059,12 @@ func (f *fakeDockerClient) ListContainerDir(_ context.Context, _ string, p strin
|
||||
}
|
||||
if f.listDirFailMap != nil {
|
||||
if fails, ok := f.listDirFailMap[p]; ok {
|
||||
return docker.ContainerDirListing{Files: f.listDirMap[p], Failures: fails}, nil
|
||||
return docker.ContainerDirListing{Files: f.listDirMap[p], Failures: fails, Truncated: f.listDirTruncated[p]}, nil
|
||||
}
|
||||
}
|
||||
if f.listDirMap != nil {
|
||||
if dir, ok := f.listDirMap[p]; ok {
|
||||
return docker.ContainerDirListing{Files: dir}, nil
|
||||
return docker.ContainerDirListing{Files: dir, Truncated: f.listDirTruncated[p]}, nil
|
||||
}
|
||||
}
|
||||
return docker.ContainerDirListing{Files: f.listDir}, f.listDirErr
|
||||
|
||||
@@ -138,6 +138,7 @@ function FlowFilesPullDialogForm({ cachedFiles, flowId, onClose, onSuccess }: Fl
|
||||
files,
|
||||
isLoading: isListingLoading,
|
||||
refetch: refetchListing,
|
||||
truncated: isListingTruncated,
|
||||
} = useFlowContainerFiles({ flowId, paths: listingPaths });
|
||||
|
||||
/**
|
||||
@@ -468,6 +469,17 @@ function FlowFilesPullDialogForm({ cachedFiles, flowId, onClose, onSuccess }: Fl
|
||||
</Alert>
|
||||
)}
|
||||
|
||||
{isListingTruncated && (
|
||||
<Alert>
|
||||
<TriangleAlert />
|
||||
<AlertTitle>Directory truncated</AlertTitle>
|
||||
<AlertDescription>
|
||||
This directory has too many entries to list in full; only the first {files.length} are
|
||||
shown. Open a subfolder to see the rest.
|
||||
</AlertDescription>
|
||||
</Alert>
|
||||
)}
|
||||
|
||||
<FileManager
|
||||
bulkActions={bulkActions}
|
||||
className="h-[360px]"
|
||||
|
||||
@@ -21,6 +21,8 @@ export interface ContainerFilesResponse {
|
||||
files: RestContainerFile[];
|
||||
path: string;
|
||||
total: number;
|
||||
/** True when the directory held more entries than the cap and only the first page was returned. */
|
||||
truncated?: boolean;
|
||||
}
|
||||
|
||||
export type FlowFile = FlowFileFragmentFragment;
|
||||
|
||||
@@ -36,6 +36,8 @@ interface UseFlowContainerFilesResult {
|
||||
* the dialog.
|
||||
*/
|
||||
refetch: () => Promise<void>;
|
||||
/** True when the directory had more entries than the cap and only the first page was returned. */
|
||||
truncated: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -54,6 +56,7 @@ interface UseFlowContainerFilesResult {
|
||||
export function useFlowContainerFiles({ flowId, paths }: UseFlowContainerFilesParams): UseFlowContainerFilesResult {
|
||||
const [files, setFiles] = useState<FileNode[]>([]);
|
||||
const [failures, setFailures] = useState<ContainerFileFailure[]>([]);
|
||||
const [truncated, setTruncated] = useState(false);
|
||||
const [isLoading, setIsLoading] = useState(false);
|
||||
const [error, setError] = useState<Error | null>(null);
|
||||
|
||||
@@ -69,11 +72,18 @@ export function useFlowContainerFiles({ flowId, paths }: UseFlowContainerFilesPa
|
||||
// when the user drills in / out faster than the server can respond).
|
||||
const currentTokenRef = useRef(0);
|
||||
|
||||
// The path key of the last fetch. A fetch clears the visible listing only when
|
||||
// the path actually changes, so an in-place refetch (Refresh / post-Pull) keeps
|
||||
// the current rows under the loading guard instead of flashing a skeleton.
|
||||
const lastPathsKeyRef = useRef<null | string>(null);
|
||||
|
||||
const fetchListing = useCallback(async () => {
|
||||
if (!flowId || paths.length === 0) {
|
||||
currentTokenRef.current += 1;
|
||||
lastPathsKeyRef.current = null;
|
||||
setFiles([]);
|
||||
setFailures([]);
|
||||
setTruncated(false);
|
||||
setIsLoading(false);
|
||||
setError(null);
|
||||
|
||||
@@ -84,10 +94,17 @@ export function useFlowContainerFiles({ flowId, paths }: UseFlowContainerFilesPa
|
||||
|
||||
setIsLoading(true);
|
||||
setError(null);
|
||||
// Clear the previous directory's data so a pending navigation never shows
|
||||
// the old listing / old failure banner attributed to the new breadcrumb.
|
||||
setFiles([]);
|
||||
setFailures([]);
|
||||
|
||||
// Only reset the listing when navigating to a different path, so a pending
|
||||
// navigation never shows the old listing under the new breadcrumb — a
|
||||
// same-path refetch keeps the current rows visible.
|
||||
if (pathsKey !== lastPathsKeyRef.current) {
|
||||
setFiles([]);
|
||||
setFailures([]);
|
||||
setTruncated(false);
|
||||
}
|
||||
|
||||
lastPathsKeyRef.current = pathsKey;
|
||||
|
||||
try {
|
||||
const url = `${FLOW_FILES_CONTAINER_API_PATH(flowId)}?${buildPathsQuery(paths)}`;
|
||||
@@ -103,6 +120,7 @@ export function useFlowContainerFiles({ flowId, paths }: UseFlowContainerFilesPa
|
||||
|
||||
setFiles(data.files.map(containerFileToFileNode));
|
||||
setFailures(data.failures ?? []);
|
||||
setTruncated(data.truncated ?? false);
|
||||
} catch (caught) {
|
||||
if (token !== currentTokenRef.current) {
|
||||
return;
|
||||
@@ -111,12 +129,13 @@ export function useFlowContainerFiles({ flowId, paths }: UseFlowContainerFilesPa
|
||||
setError(new Error(getApiErrorMessage(caught, 'Failed to load container files')));
|
||||
setFiles([]);
|
||||
setFailures([]);
|
||||
setTruncated(false);
|
||||
} finally {
|
||||
if (token === currentTokenRef.current) {
|
||||
setIsLoading(false);
|
||||
}
|
||||
}
|
||||
}, [flowId, paths]);
|
||||
}, [flowId, paths, pathsKey]);
|
||||
|
||||
useEffect(() => {
|
||||
// fetchListing is an async callback that handles its own loading state via setState
|
||||
@@ -135,5 +154,6 @@ export function useFlowContainerFiles({ flowId, paths }: UseFlowContainerFilesPa
|
||||
files,
|
||||
isLoading,
|
||||
refetch: fetchListing,
|
||||
truncated,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user