From 7b67b1c0ea817a1a8406846c33f3ebda9b5dc6ef Mon Sep 17 00:00:00 2001 From: Sergey Kozyrenko Date: Sun, 12 Jul 2026 22:38:17 +0700 Subject: [PATCH] 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 --- backend/pkg/docker/client.go | 21 ++++++++----- backend/pkg/server/docs/docs.go | 4 +++ backend/pkg/server/docs/swagger.json | 4 +++ backend/pkg/server/docs/swagger.yaml | 5 ++++ backend/pkg/server/models/flow_files.go | 3 ++ backend/pkg/server/services/flow_files.go | 13 +++++--- .../services/flow_files_partial_test.go | 30 +++++++++++++++++++ .../pkg/server/services/flow_files_test.go | 15 +++++----- .../flows/files/flow-files-pull-dialog.tsx | 12 ++++++++ .../features/flows/files/flow-files-utils.ts | 2 ++ .../flows/files/use-flow-container-files.ts | 30 +++++++++++++++---- 11 files changed, 115 insertions(+), 24 deletions(-) diff --git a/backend/pkg/docker/client.go b/backend/pkg/docker/client.go index 18ac9862..5eb7b268 100644 --- a/backend/pkg/docker/client.go +++ b/backend/pkg/docker/client.go @@ -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), diff --git a/backend/pkg/server/docs/docs.go b/backend/pkg/server/docs/docs.go index cd477efe..40cc49e4 100644 --- a/backend/pkg/server/docs/docs.go +++ b/backend/pkg/server/docs/docs.go @@ -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" } } }, diff --git a/backend/pkg/server/docs/swagger.json b/backend/pkg/server/docs/swagger.json index 989a8805..ea078cd2 100644 --- a/backend/pkg/server/docs/swagger.json +++ b/backend/pkg/server/docs/swagger.json @@ -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" } } }, diff --git a/backend/pkg/server/docs/swagger.yaml b/backend/pkg/server/docs/swagger.yaml index 3f2172a5..78dee0bf 100644 --- a/backend/pkg/server/docs/swagger.yaml +++ b/backend/pkg/server/docs/swagger.yaml @@ -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: diff --git a/backend/pkg/server/models/flow_files.go b/backend/pkg/server/models/flow_files.go index e478ff70..44a40b96 100644 --- a/backend/pkg/server/models/flow_files.go +++ b/backend/pkg/server/models/flow_files.go @@ -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. diff --git a/backend/pkg/server/services/flow_files.go b/backend/pkg/server/services/flow_files.go index cae13891..7f1a434e 100644 --- a/backend/pkg/server/services/flow_files.go +++ b/backend/pkg/server/services/flow_files.go @@ -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, }) } diff --git a/backend/pkg/server/services/flow_files_partial_test.go b/backend/pkg/server/services/flow_files_partial_test.go index 77be6fdb..ceba24c6 100644 --- a/backend/pkg/server/services/flow_files_partial_test.go +++ b/backend/pkg/server/services/flow_files_partial_test.go @@ -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) +} diff --git a/backend/pkg/server/services/flow_files_test.go b/backend/pkg/server/services/flow_files_test.go index 307fa42b..6044d2b9 100644 --- a/backend/pkg/server/services/flow_files_test.go +++ b/backend/pkg/server/services/flow_files_test.go @@ -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 diff --git a/frontend/src/features/flows/files/flow-files-pull-dialog.tsx b/frontend/src/features/flows/files/flow-files-pull-dialog.tsx index bde9f929..1aacc92a 100644 --- a/frontend/src/features/flows/files/flow-files-pull-dialog.tsx +++ b/frontend/src/features/flows/files/flow-files-pull-dialog.tsx @@ -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 )} + {isListingTruncated && ( + + + Directory truncated + + This directory has too many entries to list in full; only the first {files.length} are + shown. Open a subfolder to see the rest. + + + )} + Promise; + /** 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([]); const [failures, setFailures] = useState([]); + const [truncated, setTruncated] = useState(false); const [isLoading, setIsLoading] = useState(false); const [error, setError] = useState(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); + 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, }; }