From 08776a87f23a8935421666c788c9694049d6a65f Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Tue, 28 Apr 2026 12:01:12 +0200 Subject: [PATCH 1/7] merged onHttpsRequest with onHttpsRequestAsync, removed useless onError function --- functions/src/common/http-error.ts | 30 ++++++++ functions/src/handlers.ts | 107 +++++------------------------ 2 files changed, 46 insertions(+), 91 deletions(-) create mode 100644 functions/src/common/http-error.ts diff --git a/functions/src/common/http-error.ts b/functions/src/common/http-error.ts new file mode 100644 index 000000000..87af73102 --- /dev/null +++ b/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'; + } +} diff --git a/functions/src/handlers.ts b/functions/src/handlers.ts index ff95dd7f5..1b14caae6 100644 --- a/functions/src/handlers.ts +++ b/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,13 +59,6 @@ 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, @@ -72,7 +66,11 @@ export type HttpsRequestHandler = ( ) => Promise; /** - * 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, - user: DecodedIdToken, - done: () => void, - error: ErrorHandler -) => void; - -export async function invokeCallbackAsync( - callback: HttpsRequestCallback, - req: Request, - res: Response, - 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, - 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); - }) - ) - ) - ); -} From f14cab983973e1918c0f3d7fb2f04c97053df222 Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Tue, 28 Apr 2026 12:03:18 +0200 Subject: [PATCH 2/7] changed importGeoJson from callback to a handler --- functions/src/import-geojson.spec.ts | 44 ++++++++++++---------------- functions/src/import-geojson.ts | 8 ++--- functions/src/index.ts | 6 ++-- 3 files changed, 24 insertions(+), 34 deletions(-) diff --git a/functions/src/import-geojson.spec.ts b/functions/src/import-geojson.spec.ts index b656babcb..72e77572c 100644 --- a/functions/src/import-geojson.spec.ts +++ b/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,22 +255,25 @@ 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 }) => @@ -278,15 +281,16 @@ describe('importGeoJson()', () => { // 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); }); }); diff --git a/functions/src/import-geojson.ts b/functions/src/import-geojson.ts index 7816190ca..e9d073de8 100644 --- a/functions/src/import-geojson.ts +++ b/functions/src/import-geojson.ts @@ -38,13 +38,11 @@ class BadRequestError extends Error { * Read the body of a multipart HTTP POSTed form containing a GeoJson 'file' * and required 'survey' id and 'job' id to the database. */ -export function importGeoJsonCallback( +export function importGeoJsonHandler( req: Request, res: Response, - user: DecodedIdToken, - done: () => void, - error: ErrorHandler -) { + user: DecodedIdToken +): Promise { if (req.method !== 'POST') { return error( StatusCodes.METHOD_NOT_ALLOWED, diff --git a/functions/src/index.ts b/functions/src/index.ts index 0e7595bfd..e1bb91ce1 100644 --- a/functions/src/index.ts +++ b/functions/src/index.ts @@ -20,10 +20,10 @@ import { onDocumentCreated, onDocumentWritten, } from 'firebase-functions/v2/firestore'; -import { onHttpsRequest, onHttpsRequestAsync } from './handlers'; +import { onHttpsRequest } from './handlers'; import { handleProfileRefresh } from './profile-refresh'; import { sessionLoginHandler } from './session-login'; -import { importGeoJsonCallback } from './import-geojson'; +import { importGeoJsonHandler } from './import-geojson'; import { exportCsvHandler } from './export-csv'; import { exportGeojsonHandler } from './export-geojson'; import { cleanTempHandler } from './clean-temp'; @@ -70,7 +70,7 @@ export const onCreatePasslistEntry = onDocumentCreated( onCreatePasslistEntryHandler ); -export const importGeoJson = onHttpsRequestAsync(importGeoJsonCallback, { +export const importGeoJson = onHttpsRequest(importGeoJsonHandler, { memory: '4GiB', timeoutSeconds: 3600, cpu: 2, From 58f619e7c62a37dff29364169281cd0f5b487dcd Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Tue, 28 Apr 2026 12:04:20 +0200 Subject: [PATCH 3/7] use httperror instead of set response status --- functions/src/session-login.spec.ts | 12 ++++++++---- functions/src/session-login.ts | 9 +++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/functions/src/session-login.spec.ts b/functions/src/session-login.spec.ts index f805b08dd..14588fd1e 100644 --- a/functions/src/session-login.spec.ts +++ b/functions/src/session-login.spec.ts @@ -20,6 +20,7 @@ import { Request } from 'firebase-functions/v2/https'; import type { Response } from 'express'; import { StatusCodes } from 'http-status-codes'; import { SESSION_COOKIE_DURATION_MS } from '@ground/lib'; +import { HttpError } from './common/http-error'; import { sessionLoginHandler } from './session-login'; describe('sessionLoginHandler', () => { @@ -77,11 +78,14 @@ describe('sessionLoginHandler', () => { ); }); - it('returns 401 on auth error', async () => { + it('throws HttpError(401) on auth error', async () => { createSessionCookieSpy.and.rejectWith(new Error('invalid token')); - await sessionLoginHandler(req, res); - - expect(res.status).toHaveBeenCalledWith(StatusCodes.UNAUTHORIZED); + await expectAsync(sessionLoginHandler(req, res)).toBeRejectedWith( + jasmine.objectContaining({ + statusCode: StatusCodes.UNAUTHORIZED, + name: 'HttpError', + }) as unknown as HttpError + ); }); }); diff --git a/functions/src/session-login.ts b/functions/src/session-login.ts index 3f348ac6b..6e146bdcb 100644 --- a/functions/src/session-login.ts +++ b/functions/src/session-login.ts @@ -19,19 +19,20 @@ import * as logger from 'firebase-functions/logger'; import type { Response } from 'express'; import { setSessionCookie } from './common/auth'; import { StatusCodes } from 'http-status-codes'; +import { HttpError } from './common/http-error'; /** * Generates and sets a session cookie for the current user. */ export async function sessionLoginHandler(req: Request, res: Response) { + // Required for CDN: + // https://stackoverflow.com/questions/44929653/firebase-cloud-function-wont-store-cookie-named-other-than-session/44935288#44935288 + res.setHeader('Cache-Control', 'private'); try { - // Required for CDN: - // https://stackoverflow.com/questions/44929653/firebase-cloud-function-wont-store-cookie-named-other-than-session/44935288#44935288 - res.setHeader('Cache-Control', 'private'); const expiresAt = await setSessionCookie(req, res); res.json({ expiresAt }); } catch (err) { logger.error(err); - res.status(StatusCodes.UNAUTHORIZED).send('Authorization error'); + throw new HttpError(StatusCodes.UNAUTHORIZED, 'Authorization error'); } } From c4a19e18afb0b8151ee1b0bd3be9becaa6ca7629 Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Tue, 28 Apr 2026 12:04:46 +0200 Subject: [PATCH 4/7] use httperror instead of set response status --- functions/src/export-csv.ts | 26 +++--- functions/src/export-geojson.ts | 26 +++--- functions/src/import-geojson.ts | 157 +++++++++++++++++--------------- 3 files changed, 109 insertions(+), 100 deletions(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 0656af162..eca5fce40 100644 --- a/functions/src/export-csv.ts +++ b/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; diff --git a/functions/src/export-geojson.ts b/functions/src/export-geojson.ts index 0ac8643d2..9252d0eb7 100644 --- a/functions/src/export-geojson.ts +++ b/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; diff --git a/functions/src/import-geojson.ts b/functions/src/import-geojson.ts index e9d073de8..a1e1ab583 100644 --- a/functions/src/import-geojson.ts +++ b/functions/src/import-geojson.ts @@ -26,7 +26,7 @@ import { DocumentData } from 'firebase-admin/firestore'; import { GroundProtos } from '@ground/proto'; import { isGeometryValid, toDocumentData, toGeometryPb } from '@ground/lib'; import { Feature, GeoJsonProperties } from 'geojson'; -import { ErrorHandler } from './handlers'; +import { HttpError } from './common/http-error'; import Pb = GroundProtos.ground.v1beta1; @@ -44,7 +44,7 @@ export function importGeoJsonHandler( user: DecodedIdToken ): Promise { if (req.method !== 'POST') { - return error( + throw new HttpError( StatusCodes.METHOD_NOT_ALLOWED, `Expected method POST, got ${req.method}` ); @@ -52,8 +52,6 @@ export function importGeoJsonHandler( const busboy = Busboy({ headers: req.headers }); - let hasError = false; - // Dictionary used to accumulate task step values, keyed by step name. const params: { [name: string]: string } = {}; @@ -64,81 +62,96 @@ export function importGeoJsonHandler( const ownerId = user.uid; - // This code will process each file uploaded. - busboy.on('file', async (_fieldname, fileStream) => { - try { - const { survey: surveyId, job: jobId } = params; - if (!surveyId || !jobId) { - return error(StatusCodes.BAD_REQUEST, 'Missing survey and/or job ID'); - } - const survey = await db.fetchSurvey(surveyId); - if (!survey.exists) { - return error(StatusCodes.NOT_FOUND, `Survey ${surveyId} not found`); - } - if (!canImport(user, survey)) { - return error( - StatusCodes.FORBIDDEN, - `User does not have permission to import into survey ${surveyId}` - ); - } + return new Promise((resolve, reject) => { + let settled = false; - console.debug( - `Importing GeoJSON into survey '${surveyId}', job '${jobId}'` - ); + function fail(code: number, message: string) { + if (settled) return; + settled = true; + reject(new HttpError(code, message)); + } - const parser = JSONStream.parse(['features', true], undefined); - - fileStream.pipe( - parser - .on('header', (data: any) => { - try { - onGeoJsonType(data.type); - if (data.crs) onGeoJsonCrs(data.crs); - } catch (error: any) { - busboy.emit('error', error); - } - }) - .on('data', (data: any) => { - if (!hasError) onGeoJsonFeature(data, surveyId, jobId); - }) - ); - } catch (err) { - busboy.emit('error', err); + function succeed() { + if (settled) return; + settled = true; + resolve(); } - }); - // Handle non-file fields in the task. survey and job must appear - // before the file for the file handler to work properly. - busboy.on('field', (key, val) => { - params[key] = val; - }); + // This code will process each file uploaded. + busboy.on('file', async (_fieldname, fileStream) => { + try { + const { survey: surveyId, job: jobId } = params; + if (!surveyId || !jobId) { + return fail(StatusCodes.BAD_REQUEST, 'Missing survey and/or job ID'); + } + const survey = await db.fetchSurvey(surveyId); + if (!survey.exists) { + return fail(StatusCodes.NOT_FOUND, `Survey ${surveyId} not found`); + } + if (!canImport(user, survey)) { + return fail( + StatusCodes.FORBIDDEN, + `User does not have permission to import into survey ${surveyId}` + ); + } - // Triggered once all uploaded files are processed by Busboy. - busboy.on('finish', async () => { - if (hasError) return; - try { - await db.insertLocationsOfInterest(params.survey, loiDocs); - const count = loiDocs.length; - console.debug(`${count} LOIs imported`); - res.send(JSON.stringify({ count })); - done(); - } catch (err) { - console.debug(err); - error(StatusCodes.BAD_REQUEST, (err as Error).message); - } - }); + console.debug( + `Importing GeoJSON into survey '${surveyId}', job '${jobId}'` + ); - busboy.on('error', (err: any) => { - console.error('Busboy error', err); - hasError = true; - req.unpipe(busboy); - error(err.statusCode || StatusCodes.INTERNAL_SERVER_ERROR, err.message); - }); + const parser = JSONStream.parse(['features', true], undefined); + + fileStream.pipe( + parser + .on('header', (data: any) => { + try { + onGeoJsonType(data.type); + if (data.crs) onGeoJsonCrs(data.crs); + } catch (error: any) { + busboy.emit('error', error); + } + }) + .on('data', (data: any) => { + if (!settled) onGeoJsonFeature(data, surveyId, jobId); + }) + ); + } catch (err) { + busboy.emit('error', err); + } + }); + + // Handle non-file fields in the task. survey and job must appear + // before the file for the file handler to work properly. + busboy.on('field', (key, val) => { + params[key] = val; + }); + + // Triggered once all uploaded files are processed by Busboy. + busboy.on('finish', async () => { + if (settled) return; + try { + await db.insertLocationsOfInterest(params.survey, loiDocs); + const count = loiDocs.length; + console.debug(`${count} LOIs imported`); + res.send(JSON.stringify({ count })); + succeed(); + } catch (err) { + console.debug(err); + fail(StatusCodes.BAD_REQUEST, (err as Error).message); + } + }); + + busboy.on('error', (err: any) => { + console.error('Busboy error', err); + req.unpipe(busboy); + fail(err.statusCode || StatusCodes.INTERNAL_SERVER_ERROR, err.message); + }); - // Start processing the body data. - // Use this for Cloud Functions rather than `req.pipe(busboy)`: - // https://github.com/mscdex/busboy/issues/229#issuecomment-648303108 - busboy.end(req.rawBody); + // Start processing the body data. + // Use this for Cloud Functions rather than `req.pipe(busboy)`: + // https://github.com/mscdex/busboy/issues/229#issuecomment-648303108 + busboy.end(req.rawBody); + }); /** * This function is called by Busboy during file parsing to ensure that the GeoJSON From fe91b5cbebef446f7a7df160f401f15073ffa173 Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Tue, 28 Apr 2026 14:01:37 +0200 Subject: [PATCH 5/7] removed redundant bad_request error --- functions/src/import-geojson.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/functions/src/import-geojson.ts b/functions/src/import-geojson.ts index a1e1ab583..6e316855c 100644 --- a/functions/src/import-geojson.ts +++ b/functions/src/import-geojson.ts @@ -30,10 +30,6 @@ import { HttpError } from './common/http-error'; import Pb = GroundProtos.ground.v1beta1; -class BadRequestError extends Error { - statusCode = StatusCodes.BAD_REQUEST; -} - /** * Read the body of a multipart HTTP POSTed form containing a GeoJson 'file' * and required 'survey' id and 'job' id to the database. @@ -160,10 +156,14 @@ export function importGeoJsonHandler( */ function onGeoJsonType(geoJsonType: string | undefined) { if (!geoJsonType) - throw new BadRequestError('Invalid GeoJSON: Missing "type" property'); + throw new HttpError( + StatusCodes.BAD_REQUEST, + 'Invalid GeoJSON: Missing "type" property' + ); if (geoJsonType !== 'FeatureCollection') { - throw new BadRequestError( + throw new HttpError( + StatusCodes.BAD_REQUEST, `Unsupported GeoJSON Type: Expected 'FeatureCollection', got '${geoJsonType}'` ); } @@ -186,7 +186,8 @@ export function importGeoJsonHandler( } } if (!crs.endsWith('CRS84')) - throw new BadRequestError( + throw new HttpError( + StatusCodes.BAD_REQUEST, `Unsupported GeoJSON CRS: Expected 'CRS84', got '${JSON.stringify( geoJsonCrs )}'` From 61a01dcb662968336264bc975a28e8d5363f5ca9 Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Tue, 28 Apr 2026 14:09:15 +0200 Subject: [PATCH 6/7] make importGeoJsonHandler async for uniform error path --- functions/src/import-geojson.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/functions/src/import-geojson.ts b/functions/src/import-geojson.ts index 6e316855c..5cc18fadf 100644 --- a/functions/src/import-geojson.ts +++ b/functions/src/import-geojson.ts @@ -34,7 +34,7 @@ import Pb = GroundProtos.ground.v1beta1; * Read the body of a multipart HTTP POSTed form containing a GeoJson 'file' * and required 'survey' id and 'job' id to the database. */ -export function importGeoJsonHandler( +export async function importGeoJsonHandler( req: Request, res: Response, user: DecodedIdToken @@ -58,7 +58,7 @@ export function importGeoJsonHandler( const ownerId = user.uid; - return new Promise((resolve, reject) => { + await new Promise((resolve, reject) => { let settled = false; function fail(code: number, message: string) { From 776be0fb8ff07ca32c35ce14a340b4fe08513ed8 Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Tue, 28 Apr 2026 14:11:19 +0200 Subject: [PATCH 7/7] use res.json for importGeoJson response body --- functions/src/import-geojson.ts | 2 +- functions/src/testing/http-test-helpers.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/functions/src/import-geojson.ts b/functions/src/import-geojson.ts index 5cc18fadf..ee39fa198 100644 --- a/functions/src/import-geojson.ts +++ b/functions/src/import-geojson.ts @@ -129,7 +129,7 @@ export async function importGeoJsonHandler( await db.insertLocationsOfInterest(params.survey, loiDocs); const count = loiDocs.length; console.debug(`${count} LOIs imported`); - res.send(JSON.stringify({ count })); + res.json({ count }); succeed(); } catch (err) { console.debug(err); diff --git a/functions/src/testing/http-test-helpers.ts b/functions/src/testing/http-test-helpers.ts index f19b4cdc3..e45cc48f0 100644 --- a/functions/src/testing/http-test-helpers.ts +++ b/functions/src/testing/http-test-helpers.ts @@ -43,6 +43,7 @@ export async function createGetRequestSpy(args: object): Promise { export function createResponseSpy(chunks?: string[]): Response { const res = jasmine.createSpyObj>('response', [ 'send', + 'json', 'status', 'end', 'write',