Move HTTP status handling out of handlers

functions/src/common/http-error.ts

@@ -0,0 +1,30 @@
+/**
+ * Copyright 2026 The Ground Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the 'License');
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an 'AS IS' BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * Thrown by handlers to abort with a specific HTTP status code and message.
+ * The HTTPS wrapper translates these into HTTP responses; handlers should not
+ * call `res.status(...)` directly for error paths.
+ */
+export class HttpError extends Error {
+  constructor(
+    public readonly statusCode: number,
+    message: string
+  ) {
+    super(message);
+    this.name = 'HttpError';
+  }
+}

functions/src/export-csv.ts

@@ -33,6 +33,7 @@ import { List } from 'immutable';
 import { registry, timestampToInt, toMessage } from '@ground/lib';
 import { GroundProtos } from '@ground/proto';
 import { toGeoJsonGeometry } from '@ground/lib';
+import { HttpError } from './common/http-error';
 
 import Pb = GroundProtos.ground.v1beta1;
 
@@ -54,32 +55,29 @@ export async function exportCsvHandler(
 
   const surveyDoc = await db.fetchSurvey(surveyId);
   if (!surveyDoc.exists) {
-    res.status(StatusCodes.NOT_FOUND).send('Survey not found');
-    return;
+    throw new HttpError(StatusCodes.NOT_FOUND, 'Survey not found');
   }
   if (!canExport(user, surveyDoc)) {
-    res.status(StatusCodes.FORBIDDEN).send('Permission denied');
-    return;
+    throw new HttpError(StatusCodes.FORBIDDEN, 'Permission denied');
   }
   const survey = toMessage(surveyDoc.data()!, Pb.Survey);
   if (survey instanceof Error) {
-    res
-      .status(StatusCodes.INTERNAL_SERVER_ERROR)
-      .send('Unsupported or corrupt survey');
-    return;
+    throw new HttpError(
+      StatusCodes.INTERNAL_SERVER_ERROR,
+      'Unsupported or corrupt survey'
+    );
   }
 
   const jobDoc = await db.fetchJob(surveyId, jobId);
   if (!jobDoc.exists || !jobDoc.data()) {
-    res.status(StatusCodes.NOT_FOUND).send('Job not found');
-    return;
+    throw new HttpError(StatusCodes.NOT_FOUND, 'Job not found');
   }
   const job = toMessage(jobDoc.data()!, Pb.Job);
   if (job instanceof Error) {
-    res
-      .status(StatusCodes.INTERNAL_SERVER_ERROR)
-      .send('Unsupported or corrupt job');
-    return;
+    throw new HttpError(
+      StatusCodes.INTERNAL_SERVER_ERROR,
+      'Unsupported or corrupt job'
+    );
   }
   const { name: jobName } = job;
 

functions/src/export-geojson.ts

@@ -29,6 +29,7 @@ import { StatusCodes } from 'http-status-codes';
 import { toMessage } from '@ground/lib';
 import { GroundProtos } from '@ground/proto';
 import { toGeoJsonGeometry } from '@ground/lib';
+import { HttpError } from './common/http-error';
 
 import Pb = GroundProtos.ground.v1beta1;
 
@@ -47,32 +48,29 @@ export async function exportGeojsonHandler(
 
   const surveyDoc = await db.fetchSurvey(surveyId);
   if (!surveyDoc.exists) {
-    res.status(StatusCodes.NOT_FOUND).send('Survey not found');
-    return;
+    throw new HttpError(StatusCodes.NOT_FOUND, 'Survey not found');
   }
   if (!canExport(user, surveyDoc)) {
-    res.status(StatusCodes.FORBIDDEN).send('Permission denied');
-    return;
+    throw new HttpError(StatusCodes.FORBIDDEN, 'Permission denied');
   }
   const survey = toMessage(surveyDoc.data()!, Pb.Survey);
   if (survey instanceof Error) {
-    res
-      .status(StatusCodes.INTERNAL_SERVER_ERROR)
-      .send('Unsupported or corrupt survey');
-    return;
+    throw new HttpError(
+      StatusCodes.INTERNAL_SERVER_ERROR,
+      'Unsupported or corrupt survey'
+    );
   }
 
   const jobDoc = await db.fetchJob(surveyId, jobId);
   if (!jobDoc.exists || !jobDoc.data()) {
-    res.status(StatusCodes.NOT_FOUND).send('Job not found');
-    return;
+    throw new HttpError(StatusCodes.NOT_FOUND, 'Job not found');
   }
   const job = toMessage(jobDoc.data()!, Pb.Job);
   if (job instanceof Error) {
-    res
-      .status(StatusCodes.INTERNAL_SERVER_ERROR)
-      .send('Unsupported or corrupt job');
-    return;
+    throw new HttpError(
+      StatusCodes.INTERNAL_SERVER_ERROR,
+      'Unsupported or corrupt job'
+    );
   }
   const { name: jobName } = job;
 

functions/src/handlers.ts

@@ -20,6 +20,7 @@ import { HttpsOptions, Request, onRequest } from 'firebase-functions/v2/https';
 import type { Response } from 'express';
 import { StatusCodes } from 'http-status-codes';
 import { getDecodedIdToken } from './common/auth';
+import { HttpError } from './common/http-error';
 import cookieParser from 'cookie-parser';
 
 const corsOptions = { origin: true };
@@ -58,21 +59,18 @@ async function requireIdToken(
   }
 }
 
-function onError(res: any, err: any) {
-  console.error(err);
-  res
-    .status(StatusCodes.INTERNAL_SERVER_ERROR)
-    .end(`Internal error: ${err.message}`);
-}
-
 export type HttpsRequestHandler = (
   req: Request,
   res: Response,
   idToken: DecodedIdToken
 ) => Promise<any>;
 
 /**
- * A synchronous HTTPS request handler. The HTTPS request is closed as soon as the handler resolves.
+ * Wraps an async HTTPS request handler with CORS, cookie parsing, and ID-token
+ * authentication. Translates `HttpError` thrown by the handler into the
+ * corresponding HTTP response. Generic errors become 500. On success, sends
+ * 200 OK if the handler did not already write headers (e.g. via `res.send`,
+ * `res.json`, `res.redirect`, or streaming).
  */
 export function onHttpsRequest(
   handler: HttpsRequestHandler,
@@ -87,92 +85,19 @@ export function onHttpsRequest(
           await requireIdToken(req, res, async (idToken: DecodedIdToken) => {
             try {
               await handler(req, res, idToken);
-            } catch (error) {
-              onError(res, error);
+              if (!res.headersSent) res.status(StatusCodes.OK).end();
+            } catch (err) {
+              if (err instanceof HttpError) {
+                res.status(err.statusCode).send(err.message);
+              } else {
+                console.error(err);
+                res
+                  .status(StatusCodes.INTERNAL_SERVER_ERROR)
+                  .end(`Internal error: ${(err as Error).message}`);
+              }
             }
           })
       )
     )
   );
 }
-
-/** A function which is to be called by HTTPS callbacks on failure. */
-export type ErrorHandler = (httpStatusCode: number, message: string) => void;
-
-/**
- * A callback-based HTTPS request handler. Functions of this type are expected to call
- * `done()` on completion or `error()` on failure. The function itself may return before
- * work is completed, but the HTTPS request will not complete until one of those two
- * callbacks are invoked.
- */
-export type HttpsRequestCallback = (
-  req: Request,
-  res: Response<any>,
-  user: DecodedIdToken,
-  done: () => void,
-  error: ErrorHandler
-) => void;
-
-export async function invokeCallbackAsync(
-  callback: HttpsRequestCallback,
-  req: Request,
-  res: Response<any>,
-  user: DecodedIdToken
-) {
-  await new Promise((resolve, reject) =>
-    invokeCallback(
-      callback,
-      req,
-      res,
-      user,
-      () => {
-        res.status(StatusCodes.OK).end();
-        resolve(undefined);
-      },
-      (errorCode: number, message: string) => {
-        res.status(errorCode).end(message);
-        reject(`${message} (HTTP status ${errorCode})`);
-      }
-    )
-  );
-}
-
-function invokeCallback(
-  callback: HttpsRequestCallback,
-  req: Request,
-  res: Response<any>,
-  user: DecodedIdToken,
-  done: () => void,
-  error: ErrorHandler
-) {
-  try {
-    callback(req, res, user, done, error);
-  } catch (e: any) {
-    console.error('Unhandled exception', e);
-    error(StatusCodes.INTERNAL_SERVER_ERROR, e.toString());
-  }
-}
-
-/**
- * Call an asynchronous HTTPS request handler. Handlers of this type are expected to call
- * `done()` on completion or `error()` on failure. The handler itself may return before
- * work is completed, but the HTTPS request will not complete until one of those two
- * callbacks are invoked.
- */
-export function onHttpsRequestAsync(
-  callback: HttpsRequestCallback,
-  options: HttpsOptions = {}
-) {
-  return onRequest(options, (req: Request, res: Response) =>
-    corsMiddleware(req, res, () =>
-      cookieParser()(
-        req as any,
-        res as any,
-        async () =>
-          await requireIdToken(req, res, async (idToken: DecodedIdToken) => {
-            await invokeCallbackAsync(callback, req, res, idToken);
-          })
-      )
-    )
-  );
-}

functions/src/import-geojson.spec.ts

@@ -22,11 +22,11 @@ import {
   createPostRequestSpy,
   createResponseSpy,
 } from './testing/http-test-helpers';
-import { importGeoJsonCallback } from './import-geojson';
+import { importGeoJsonHandler } from './import-geojson';
 import { DecodedIdToken } from 'firebase-admin/auth';
 import { Blob, FormData } from 'formdata-node';
 import { StatusCodes } from 'http-status-codes';
-import { invokeCallbackAsync } from './handlers';
+import { HttpError } from './common/http-error';
 import { SURVEY_ORGANIZER_ROLE } from './common/auth';
 import { getDatastore, resetDatastore } from './common/context';
 import { Firestore } from 'firebase-admin/firestore';
@@ -255,38 +255,42 @@ describe('importGeoJson()', () => {
     return form;
   }
 
-  async function runImport(geoJson: object) {
+  async function runImport(geoJson: object): Promise<{
+    thrownStatusCode?: number;
+  }> {
     const req = await createPostRequestSpy(
       { url: '/importGeoJson' },
       createPostData(surveyId, jobId, geoJson)
     );
     const res = createResponseSpy();
     try {
-      // Ideally we would call `importGeoJson` directly rather than via `invokeCallbackAsync`,
-      // but that would require mocking all middleware which may be overkill.
-      await invokeCallbackAsync(importGeoJsonCallback, req, res, {
-        email,
-      } as DecodedIdToken);
+      await importGeoJsonHandler(req, res, { email } as DecodedIdToken);
+      return {};
     } catch (err) {
-      console.log(err);
+      return {
+        thrownStatusCode:
+          err instanceof HttpError
+            ? err.statusCode
+            : StatusCodes.INTERNAL_SERVER_ERROR,
+      };
     }
-    return res;
   }
 
   testCases.forEach(({ desc, input, expectedStatus, expected }) =>
     it(desc, async () => {
       // Add survey.
       mockFirestore.doc(`surveys/${surveyId}`).set(survey);
 
-      const res = await runImport(input);
+      const { thrownStatusCode } = await runImport(input);
 
-      expect(res.status).toHaveBeenCalledOnceWith(expectedStatus);
       if (expectedStatus === StatusCodes.OK) {
+        expect(thrownStatusCode).toBeUndefined();
         expect(insertLocationsOfInterestSpy).toHaveBeenCalledOnceWith(
           surveyId,
           expected
         );
       } else {
+        expect(thrownStatusCode).toBe(expectedStatus);
         expect(insertLocationsOfInterestSpy).not.toHaveBeenCalled();
       }
     })
@@ -332,20 +336,8 @@ describe('importGeoJson()', () => {
       Promise.reject(new Error('Database connection failed'))
     );
 
-    const req = await createPostRequestSpy(
-      { url: '/importGeoJson' },
-      createPostData(surveyId, jobId, geoJsonWithPoint)
-    );
-    const res = createResponseSpy();
-
-    try {
-      await invokeCallbackAsync(importGeoJsonCallback, req, res, {
-        email,
-      } as DecodedIdToken);
-    } catch {
-      // Expected to reject.
-    }
+    const { thrownStatusCode } = await runImport(geoJsonWithPoint);
 
-    expect(res.status).toHaveBeenCalledWith(StatusCodes.INTERNAL_SERVER_ERROR);
+    expect(thrownStatusCode).toBe(StatusCodes.INTERNAL_SERVER_ERROR);
   });
 });