#952 Improve dataset endpoint error catching

Author: tariqksoliman

API/Backend/Datasets/routes/datasets.js

@@ -299,7 +299,15 @@ router.post("/upload", function (req, res, next) {
     let working = false;
     let uploaded = "";
     let uploadFinished = false;
-    const busboy = new Busboy({ headers: req.headers });
+    let busboy;
+    try {
+      busboy = new Busboy({ headers: req.headers });
+    } catch (err) {
+      return res.status(400).send({
+        status: "failure",
+        message: "Invalid Content-Type for file upload.",
+      });
+    }
     busboy.on("file", function (fieldname, file, filename, encoding, mimetype) {
       file.on("data", function (data) {
         uploaded += data.toString("utf8");
@@ -330,6 +338,15 @@ router.post("/upload", function (req, res, next) {
       uploadFinished = true;
       populateInterval = setInterval(populateNext, 100);
     });
+    busboy.on("error", function (err) {
+      logger("error", "Busboy error during dataset upload.", req.originalUrl, req, err);
+      if (!res.headersSent) {
+        res.status(400).send({
+          status: "failure",
+          message: "Error parsing upload.",
+        });
+      }
+    });
     req.pipe(busboy);
 
     let totalPopulates = 0;

tests/e2e/api/datasets.spec.js

@@ -1,4 +1,5 @@
 import { test, expect } from '@playwright/test';
+import { getLongTermToken, revokeLongTermToken } from '../../helpers/auth.js';
 
 /**
  * E2E tests for the Datasets API.
@@ -131,13 +132,88 @@ test.describe.serial('Datasets API — recreate and query lifecycle', () => {
   });
 });
 
-test.describe('Datasets API — upload', () => {
+test.describe.serial('Datasets API — upload', () => {
   const baseURL = process.env.TEST_BASE_URL || 'http://localhost:18888';
+  let longTermToken = null;
 
-  test('POST /api/datasets/upload — multipart CSV upload', async ({ request }) => {
-    // Upload requires isLongTermToken (API key auth), which the test user may not have.
-    // We still validate the endpoint doesn't crash.
-    test.skip(true, 'SKIP: Dataset upload requires long-term API token (isLongTermToken) — not available in standard test auth');
+  test.beforeAll(async ({ request }) => {
+    longTermToken = await getLongTermToken(request);
+  });
+
+  test.afterAll(async ({ request }) => {
+    if (longTermToken) await revokeLongTermToken(request, longTermToken);
+  });
+
+  // --- Crash regression tests (busboy error handling) ---
+
+  test('POST /api/datasets/upload — invalid Content-Type does not crash server (returns 400, not 500)', async ({ request }) => {
+    // Sending a non-multipart Content-Type previously caused busboy to throw
+    // synchronously with no try/catch, crashing the process. After the fix it
+    // must return 400 (when a valid token is present) or a non-5xx auth failure.
+    const response = await request.post(`${baseURL}/api/datasets/upload`, {
+      headers: {
+        ...(longTermToken ? { Authorization: `Bearer ${longTermToken}` } : {}),
+        'Content-Type': 'text/plain',
+      },
+      data: 'not a multipart body',
+    });
+    expect(response.status()).not.toBe(500);
+    if (longTermToken) {
+      expect(response.status()).toBe(400);
+      const body = await response.json();
+      expect(body.status).toBe('failure');
+    }
+  });
+
+  test('POST /api/datasets/upload — missing Content-Type does not crash server', async ({ request }) => {
+    // Busboy throws 'Missing Content-Type' synchronously when the header is absent.
+    const response = await request.post(`${baseURL}/api/datasets/upload`, {
+      headers: {
+        ...(longTermToken ? { Authorization: `Bearer ${longTermToken}` } : {}),
+        'Content-Type': '',
+      },
+      data: '',
+    });
+    expect(response.status()).not.toBe(500);
+  });
+
+  // --- Auth gate ---
+
+  test('POST /api/datasets/upload — without token returns failure, not 5xx', async ({ request }) => {
+    const response = await request.post(`${baseURL}/api/datasets/upload`, {
+      multipart: {
+        name: `upload_noauth_${Date.now()}`,
+        header: JSON.stringify(['name', 'value']),
+        upsert: 'false',
+        file: { name: 'data.csv', mimeType: 'text/csv', buffer: Buffer.from('name,value\nfoo,1\n') },
+      },
+    });
+    expect(response.status()).not.toBe(500);
+    const body = await response.json();
+    expect(body.status).toBe('failure');
+  });
+
+  // --- Happy-path upload (only when a long-term token was obtained) ---
+
+  test('POST /api/datasets/upload — valid multipart CSV upload does not return 5xx', async ({ request }) => {
+    if (!longTermToken) {
+      test.skip(true, 'SKIP: Could not obtain a long-term token');
+      return;
+    }
+
+    const csvContent = ['name,score', 'alpha,10', 'beta,20', 'gamma,30'].join('\n') + '\n';
+    const response = await request.post(`${baseURL}/api/datasets/upload`, {
+      headers: { Authorization: `Bearer ${longTermToken}` },
+      multipart: {
+        name: `upload_e2e_${Date.now()}`,
+        header: JSON.stringify(['name', 'score']),
+        upsert: 'false',
+        file: { name: 'data.csv', mimeType: 'text/csv', buffer: Buffer.from(csvContent) },
+      },
+    });
+    expect(response.status()).not.toBe(500);
+    const body = await response.json();
+    expect(body).toHaveProperty('status');
   });
 });
 

tests/helpers/auth.js

@@ -91,3 +91,47 @@ export async function loginAsAdmin(request) {
     password: DEFAULT_ADMIN.password,
   });
 }
+
+/**
+ * Generate a short-lived long-term token via the admin API.
+ *
+ * Requires the server to be running with AUTH enabled and the test admin
+ * account to have admin privileges. Returns `null` if the token could not
+ * be obtained (e.g. AUTH=off, no admin account).
+ *
+ * Callers are responsible for revoking the token when done (see
+ * `revokeLongTermToken`).
+ *
+ * @param {import('@playwright/test').APIRequestContext} request
+ * @param {string} [name] - Human-readable label for the token.
+ * @returns {Promise<string|null>} The raw token string, or null.
+ */
+export async function getLongTermToken(request, name = `test_token_${Date.now()}`) {
+  const resp = await request.post('/api/longtermtoken/generate', {
+    data: { name, period: '1d' },
+  });
+  if (resp.status() !== 200) return null;
+  const body = await resp.json();
+  if (body.status !== 'success' || !body.body?.token) return null;
+  return body.body.token;
+}
+
+/**
+ * Revoke a long-term token by its token string.
+ *
+ * Looks up the token in the listing and deletes it. Silently no-ops if
+ * the token is not found or the listing call fails.
+ *
+ * @param {import('@playwright/test').APIRequestContext} request
+ * @param {string} token - The token string returned by `getLongTermToken`.
+ * @returns {Promise<void>}
+ */
+export async function revokeLongTermToken(request, token) {
+  const listResp = await request.get('/api/longtermtoken/get');
+  if (listResp.status() !== 200) return;
+  const listBody = await listResp.json();
+  if (listBody.status !== 'success') return;
+  const entry = listBody.tokens?.find((t) => t.token === token);
+  if (!entry) return;
+  await request.post('/api/longtermtoken/clear', { data: { id: entry.id } });
+}