Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 18 additions & 30 deletions packages/appkit/src/plugins/files/plugin.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { STATUS_CODES } from "node:http";
import { Readable } from "node:stream";
import { ApiError } from "@databricks/sdk-experimental";
import type express from "express";
Expand Down Expand Up @@ -424,6 +425,13 @@ export class FilesPlugin extends Plugin {
res.status(500).json({ error: fallbackMessage, plugin: this.name });
}

private _sendStatusError(res: express.Response, status: number): void {
res.status(status).json({
error: STATUS_CODES[status] ?? "Unknown Error",
plugin: this.name,
});
}

private async _handleList(
req: express.Request,
res: express.Response,
Expand All @@ -446,9 +454,7 @@ export class FilesPlugin extends Plugin {
);

if (!result.ok) {
res
.status(result.status)
.json({ error: "List failed", plugin: this.name });
this._sendStatusError(res, result.status);
return;
}
res.json(result.data);
Expand Down Expand Up @@ -484,9 +490,7 @@ export class FilesPlugin extends Plugin {
);

if (!result.ok) {
res
.status(result.status)
.json({ error: "Read failed", plugin: this.name });
this._sendStatusError(res, result.status);
return;
}
res.type("text/plain").send(result.data);
Expand Down Expand Up @@ -550,9 +554,7 @@ export class FilesPlugin extends Plugin {
}, settings);

if (!response.ok) {
res
.status(response.status)
.json({ error: `${label} failed`, plugin: this.name });
this._sendStatusError(res, response.status);
return;
}

Expand Down Expand Up @@ -588,9 +590,7 @@ export class FilesPlugin extends Plugin {
nodeStream.on("error", (err) => {
logger.error("Stream error during %s: %O", opts.mode, err);
if (!res.headersSent) {
res
.status(500)
.json({ error: `${label} failed`, plugin: this.name });
this._sendStatusError(res, 500);
} else {
res.destroy();
}
Expand Down Expand Up @@ -631,9 +631,7 @@ export class FilesPlugin extends Plugin {
);

if (!result.ok) {
res
.status(result.status)
.json({ error: "Exists check failed", plugin: this.name });
this._sendStatusError(res, result.status);
return;
}
res.json({ exists: result.data });
Expand Down Expand Up @@ -669,9 +667,7 @@ export class FilesPlugin extends Plugin {
);

if (!result.ok) {
res
.status(result.status)
.json({ error: "Metadata fetch failed", plugin: this.name });
this._sendStatusError(res, result.status);
return;
}
res.json(result.data);
Expand Down Expand Up @@ -707,9 +703,7 @@ export class FilesPlugin extends Plugin {
);

if (!result.ok) {
res
.status(result.status)
.json({ error: "Preview failed", plugin: this.name });
this._sendStatusError(res, result.status);
return;
}
res.json(result.data);
Expand Down Expand Up @@ -807,9 +801,7 @@ export class FilesPlugin extends Plugin {
path,
contentLength ?? 0,
);
res
.status(result.status)
.json({ error: "Upload failed", plugin: this.name });
this._sendStatusError(res, result.status);
return;
}

Expand Down Expand Up @@ -862,9 +854,7 @@ export class FilesPlugin extends Plugin {
);

if (!result.ok) {
res
.status(result.status)
.json({ error: "Create directory failed", plugin: this.name });
this._sendStatusError(res, result.status);
return;
}

Expand Down Expand Up @@ -909,9 +899,7 @@ export class FilesPlugin extends Plugin {
);

if (!result.ok) {
res
.status(result.status)
.json({ error: "Delete failed", plugin: this.name });
this._sendStatusError(res, result.status);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ describe("Files Plugin Integration", () => {

expect(response.status).toBe(500);
const data = (await response.json()) as { error: string; plugin: string };
expect(data.error).toBe("Metadata fetch failed");
expect(data.error).toBe("Internal Server Error");
expect(data.plugin).toBe("files");
});

Expand All @@ -583,7 +583,7 @@ describe("Files Plugin Integration", () => {

expect(response.status).toBe(500);
const data = (await response.json()) as { error: string; plugin: string };
expect(data.error).toBe("List failed");
expect(data.error).toBe("Internal Server Error");
expect(data.plugin).toBe("files");
});

Expand All @@ -602,7 +602,7 @@ describe("Files Plugin Integration", () => {
error: string;
plugin: string;
};
expect(data.error).toBe("Metadata fetch failed");
expect(data.error).toBe("Internal Server Error");
expect(data.plugin).toBe("files");
});

Expand All @@ -622,7 +622,7 @@ describe("Files Plugin Integration", () => {
error: string;
plugin: string;
};
expect(data.error).toBe("Create directory failed");
expect(data.error).toBe("Internal Server Error");
expect(data.plugin).toBe("files");
});
});
Expand Down
16 changes: 8 additions & 8 deletions packages/appkit/src/plugins/files/tests/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ describe("FilesPlugin", () => {
expect(res.status).toHaveBeenCalledWith(500);
expect(res.json).toHaveBeenCalledWith(
expect.objectContaining({
error: "List failed",
error: "Internal Server Error",
plugin: "files",
}),
);
Expand All @@ -698,7 +698,7 @@ describe("FilesPlugin", () => {

expect(res.status).toHaveBeenCalledWith(500);
expect(res.json).toHaveBeenCalledWith(
expect.objectContaining({ error: "Read failed" }),
expect.objectContaining({ error: "Internal Server Error" }),
);
});

Expand All @@ -722,7 +722,7 @@ describe("FilesPlugin", () => {

expect(res.status).toHaveBeenCalledWith(500);
expect(res.json).toHaveBeenCalledWith(
expect.objectContaining({ error: "Exists check failed" }),
expect.objectContaining({ error: "Internal Server Error" }),
);
});

Expand All @@ -746,7 +746,7 @@ describe("FilesPlugin", () => {

expect(res.status).toHaveBeenCalledWith(500);
expect(res.json).toHaveBeenCalledWith(
expect.objectContaining({ error: "Metadata fetch failed" }),
expect.objectContaining({ error: "Internal Server Error" }),
);
});

Expand All @@ -768,7 +768,7 @@ describe("FilesPlugin", () => {

expect(res.status).toHaveBeenCalledWith(500);
expect(res.json).toHaveBeenCalledWith(
expect.objectContaining({ error: "Download failed" }),
expect.objectContaining({ error: "Internal Server Error" }),
);
});

Expand All @@ -791,7 +791,7 @@ describe("FilesPlugin", () => {

expect(res.status).toHaveBeenCalledWith(500);
expect(res.json).toHaveBeenCalledWith(
expect.objectContaining({ error: "Create directory failed" }),
expect.objectContaining({ error: "Internal Server Error" }),
);
});

Expand Down Expand Up @@ -835,7 +835,7 @@ describe("FilesPlugin", () => {
await handlerPromise;

const errorBody = res.json.mock.calls[0][0];
expect(errorBody.error).toBe("List failed");
expect(errorBody.error).toBe("Internal Server Error");
expect(errorBody.error).not.toContain("secret");
expect(errorBody.error).not.toContain("internal");
});
Expand Down Expand Up @@ -876,7 +876,7 @@ describe("FilesPlugin", () => {
expect(signalWasAborted).toBe(true);
expect(res.status).toHaveBeenCalledWith(500);
expect(res.json).toHaveBeenCalledWith(
expect.objectContaining({ error: "List failed" }),
expect.objectContaining({ error: "Internal Server Error" }),
);
});

Expand Down