This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-8932-154042ae219f20e8bdf54a5f2312f988cc80e627 in repository https://gitbox.apache.org/repos/asf/texera.git
commit e5d52e062ee7a0c148db4b6e96dcc44333917f25 Author: Tanishq Gandhi <[email protected]> AuthorDate: Thu Oct 15 22:55:58 2026 +0000 fix(frontend): show an error when a file preview fails to load, and drop a superseded file (#8932) ### What changes were proposed in this PR? The file preview on dataset and model detail pages subscribed to each file request without an error handler and never cancelled it. So: A failed download (storage unreachable, expired link, network error) left the preview on “File content is loading” forever. When a file was clicked before the previous one finished, the earlier file could arrive last and replace the newer one’s content under the newer file’s name. UserDatasetFileRendererComponent now keeps the current request and cancels it whenever a new file is loaded, and shows the existing “File loading encounter error” message when the request fails. Before: a failed download keeps loading, and a slow earlier file replaces the selected one: <img width="1440" height="900" alt="before-1-download-failed" src="https://github.com/user-attachments/assets/ca24ccb0-e721-48c1-b18d-23dffc1996dc"; /> <img width="1440" height="900" alt="before-3-first-file-overwrites" src="https://github.com/user-attachments/assets/3522331f-1941-42ff-bc8a-11364342837c"; /> After: the failed download shows the error, and the selected file stays: <img width="1440" height="900" alt="after-1-download-failed" src="https://github.com/user-attachments/assets/6e3b9ded-910d-4d13-ae12-879918f82a4a"; /> <img width="1440" height="900" alt="after-2-second-file-kept" src="https://github.com/user-attachments/assets/782cf354-7963-4ebc-bdbb-4685aac03601"; /> ### Any related issues, documentation, discussions? Closes #8930 ### How was this PR tested? Added two cases to user-dataset-file-renderer.component.spec.ts: a failed fetch shows the loading error, and a slow file is dropped once another file is selected. Both fail without the fix. cd frontend yarn ng test --watch=false --include='**/user-dataset-file-renderer.component.spec.ts' Tests 57 passed (57) Also checked against a local stack: with the object store stopped, clicking a file shows the error; and with one file’s request delayed, clicking it and then another file keeps the second file’s content. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5.5) --------- Co-authored-by: Meng Wang <[email protected]> --- .../user-dataset-file-renderer.component.spec.ts | 68 +++++++++++++++++++++- .../user-dataset-file-renderer.component.ts | 25 ++++++-- 2 files changed, 86 insertions(+), 7 deletions(-) diff --git a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-file-renderer/user-dataset-file-renderer.component.spec.ts b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-file-renderer/user-dataset-file-renderer.component.spec.ts index 547235772f..ebcc482678 100644 --- a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-file-renderer/user-dataset-file-renderer.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-file-renderer/user-dataset-file-renderer.component.spec.ts @@ -26,7 +26,7 @@ import { EntityType } from "../../../../../../hub/service/hub.service"; import { NotificationService } from "../../../../../../common/service/notification/notification.service"; import { DomSanitizer } from "@angular/platform-browser"; import { commonTestProviders } from "../../../../../../common/testing/test-utils"; -import { of } from "rxjs"; +import { of, Subject, throwError } from "rxjs"; import * as Papa from "papaparse"; import JSZip from "jszip"; import readXlsxFile from "read-excel-file"; @@ -108,6 +108,40 @@ describe("UserDatasetFileRendererComponent", () => { expect(component.isLoading).toBe(false); }); + it("shows the loading error when the file cannot be fetched", () => { + vi.spyOn(TestBed.inject(DatasetService), "retrieveDatasetVersionSingleFile").mockReturnValue( + throwError(() => new Error("download failed")) + ); + component.resourceId = 1; + component.versionId = 2; + component.filePath = "notes.txt"; + + component.reloadFileContent(); + + expect(component.isLoading).toBe(false); + expect(component.isFileLoadingError).toBe(true); + }); + + it("drops a slow file once another file is selected", () => { + const slowFile = new Subject<Blob>(); + const notes = new Blob(["notes"], { type: "text/plain" }); + vi.spyOn(TestBed.inject(DatasetService), "retrieveDatasetVersionSingleFile").mockImplementation(path => + path === "data.csv" ? slowFile : of(notes) + ); + component.resourceId = 1; + component.versionId = 2; + component.filePath = "data.csv"; + component.reloadFileContent(); + + component.filePath = "notes.txt"; + component.reloadFileContent(); + slowFile.next(new Blob(["a,b\n1,2"], { type: "text/csv" })); + + expect(slowFile.observed).toBe(false); + expect(component.displayCSV).toBe(false); + expect(component.currentFile?.size).toBe(notes.size); + }); + it("fetches from the model endpoint when the file belongs to a model", () => { const datasetSpy = vi.spyOn(TestBed.inject(DatasetService), "retrieveDatasetVersionSingleFile"); const modelSpy = vi @@ -618,6 +652,38 @@ describe("UserDatasetFileRendererComponent", () => { expect(component.tableContent).toEqual([]); }); + function loadText(filePath: string): void { + vi.spyOn(TestBed.inject(DatasetService), "retrieveDatasetVersionSingleFile").mockReturnValue( + of(new Blob(["ignored"], { type: "text/plain" })) + ); + component.resourceId = 1; + component.versionId = 2; + component.filePath = filePath; + component.reloadFileContent(); + } + + it("does not show a CSV's rows once another file is selected while it is parsing", async () => { + outcome = { text: "a,b\n1,2" }; + + loadCsv(); + loadText("notes.txt"); + await vi.waitFor(() => expect(component.textContent).toBe("a,b\n1,2")); + + expect(component.displayCSV).toBe(false); + expect(component.tableDataHeader).toEqual([]); + }); + + it("does not show a text file's content once another file is selected while it is read", async () => { + outcome = { text: "hello" }; + + loadText("first.txt"); + loadCsv(); + await vi.waitFor(() => expect(component.tableDataHeader).toEqual(["hello"])); + + expect(component.displayPlainText).toBe(false); + expect(component.textContent).toBe(""); + }); + it("flags a loading error when the CSV cannot be read", async () => { const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); outcome = { fail: true }; diff --git a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-file-renderer/user-dataset-file-renderer.component.ts b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-file-renderer/user-dataset-file-renderer.component.ts index 85180685a8..d744f6dd76 100644 --- a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-file-renderer/user-dataset-file-renderer.component.ts +++ b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-file-renderer/user-dataset-file-renderer.component.ts @@ -21,6 +21,7 @@ import { Component, Input, OnChanges, OnDestroy, OnInit, SimpleChanges } from "@ import { ResourceRegistryService } from "../../../../../service/user/resource-registry/resource-registry.service"; import { EntityType } from "../../../../../../hub/service/hub.service"; import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; +import { Subscription } from "rxjs"; import * as Papa from "papaparse"; import { ParseResult } from "papaparse"; import { DomSanitizer, SafeUrl } from "@angular/platform-browser"; @@ -139,6 +140,8 @@ export class UserDatasetFileRendererComponent implements OnInit, OnChanges, OnDe // control flags public isLoading: boolean = false; + private fileLoad?: Subscription; + private loadId = 0; public isFileSizeUnloadable = false; public isFileLoadingError: boolean = false; public isFileTypePreviewUnsupported: boolean = false; @@ -195,6 +198,10 @@ export class UserDatasetFileRendererComponent implements OnInit, OnChanges, OnDe } reloadFileContent() { + this.fileLoad?.unsubscribe(); + // Parsing finishes after the download, so its callbacks check they still belong to the shown file. + const loadId = ++this.loadId; + const isCurrent = () => loadId === this.loadId; this.turnOffAllDisplay(); // Pre-check - file size @@ -213,7 +220,7 @@ export class UserDatasetFileRendererComponent implements OnInit, OnChanges, OnDe this.isLoading = true; const retrieveSingleFile = this.resourceRegistry.get(this.resourceType).retrieveSingleFile; if (retrieveSingleFile && this.resourceId && this.versionId && this.filePath != "") { - retrieveSingleFile(this.filePath, this.isLogin) + this.fileLoad = retrieveSingleFile(this.filePath, this.isLogin) .pipe(untilDestroyed(this)) .subscribe({ next: blob => { @@ -252,6 +259,7 @@ export class UserDatasetFileRendererComponent implements OnInit, OnChanges, OnDe case MIME_TYPES.MSEXCEL: readXlsxFile(blob).then(rows => { + if (!isCurrent()) return; let parsedData: string[][] = []; rows.forEach(row => { // Convert each cell in the row to a string @@ -270,11 +278,13 @@ export class UserDatasetFileRendererComponent implements OnInit, OnChanges, OnDe // Handle CSV display Papa.parse(this.currentFile, { complete: (results: ParseResult<any>) => { + if (!isCurrent()) return; if (results.data.length > 0) { this.loadTabularFile(results.data); } }, error: error => { + if (!isCurrent()) return; console.error("Error parsing file:", error); this.onFileLoadingError(); }, @@ -282,19 +292,20 @@ export class UserDatasetFileRendererComponent implements OnInit, OnChanges, OnDe break; case MIME_TYPES.MD: this.displayMarkdown = true; - this.readFileAsText(blob); + this.readFileAsText(blob, isCurrent); break; case MIME_TYPES.JSON: this.displayJson = true; - this.readFileAsText(blob); + this.readFileAsText(blob, isCurrent); break; case MIME_TYPES.TXT: default: this.displayPlainText = true; - this.readFileAsText(blob); + this.readFileAsText(blob, isCurrent); break; } }, + error: () => this.onFileLoadingError(), }); } } @@ -340,10 +351,12 @@ export class UserDatasetFileRendererComponent implements OnInit, OnChanges, OnDe return mimeType !== MIME_TYPES.OCTET_STREAM && Object.hasOwnProperty.call(MIME_TYPE_SIZE_LIMITS_MB, mimeType); } - private readFileAsText(blob: Blob) { + private readFileAsText(blob: Blob, isCurrent: () => boolean = () => true) { const txtReader = new FileReader(); txtReader.onload = (event: any) => { - this.textContent = event.target.result; + if (isCurrent()) { + this.textContent = event.target.result; + } }; txtReader.readAsText(blob); }
