Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
45 changes: 45 additions & 0 deletions spec/v1/providers/httpsAsync.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { expect } from "chai";
import * as sinon from "sinon";
import * as https from "../../../src/v1/providers/https";
import * as logger from "../../../src/logger";
import { MockRequest } from "../../fixtures/mockrequest";
import { runHandler } from "../../helper";

describe("CloudHttpsBuilder async onRequest", () => {
let loggerSpy: sinon.SinonSpy;

beforeEach(() => {
loggerSpy = sinon.spy(logger, "error");
});

afterEach(() => {
loggerSpy.restore();
});

it("should catch and log unhandled rejections in async onRequest handlers", async () => {
const error = new Error("Async error");
const fn = https.onRequest(async (_req, _res) => {

Check failure on line 21 in spec/v1/providers/httpsAsync.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24.x)

Async arrow function has no 'await' expression
throw error;
});

const req = new MockRequest({}, {});
req.method = "GET";

await runHandler(fn, req as any);

expect(loggerSpy.calledWith("Unhandled error", error)).to.be.true;
});

it("should not log if handler completes successfully", async () => {
const fn = https.onRequest(async (_req, res) => {

Check failure on line 34 in spec/v1/providers/httpsAsync.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24.x)

Async arrow function has no 'await' expression
res.send(200);
});

const req = new MockRequest({}, {});
req.method = "GET";

await runHandler(fn, req as any);

expect(loggerSpy.called).to.be.false;
});
});
45 changes: 45 additions & 0 deletions spec/v2/providers/httpsAsync.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { expect } from "chai";
import * as sinon from "sinon";
import * as https from "../../../src/v2/providers/https";
import * as logger from "../../../src/logger";
import { MockRequest } from "../../fixtures/mockrequest";
import { runHandler } from "../../helper";

describe("v2.https.onRequest async", () => {
let loggerSpy: sinon.SinonSpy;

beforeEach(() => {
loggerSpy = sinon.spy(logger, "error");
});

afterEach(() => {
loggerSpy.restore();
});

it("should catch and log unhandled rejections in async onRequest handlers", async () => {
const error = new Error("Async error v2");
const fn = https.onRequest(async (_req, _res) => {

Check failure on line 21 in spec/v2/providers/httpsAsync.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24.x)

Async arrow function has no 'await' expression
throw error;
});

const req = new MockRequest({}, {});
req.method = "GET";

await runHandler(fn, req as any);

expect(loggerSpy.calledWith("Unhandled error", error)).to.be.true;
});

it("should not log if handler completes successfully", async () => {
const fn = https.onRequest(async (_req, res) => {

Check failure on line 34 in spec/v2/providers/httpsAsync.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24.x)

Async arrow function has no 'await' expression
res.send(200);
});

const req = new MockRequest({}, {});
req.method = "GET";

await runHandler(fn, req as any);

expect(loggerSpy.called).to.be.false;
});
});
27 changes: 27 additions & 0 deletions src/common/providers/https.ts
Original file line number Diff line number Diff line change
Expand Up @@ -952,3 +952,30 @@ function wrapOnCallHandler<Req = any, Res = any, Stream = unknown>(
}
};
}

/**
* Wraps an HTTP handler with a safety net for unhandled errors.
*
* This wrapper catches both synchronous errors and rejected Promises from `async` handlers.
* Without this, an unhandled error in an `async` handler would cause the request to hang
* until the platform timeout, as Express (v4) does not await handlers.
*
* It logs the error and returns a 500 Internal Server Error to the client if the response
* headers have not yet been sent.
*
* @internal
*/
export function withErrorHandler(
handler: (req: Request, res: express.Response) => void | Promise<void>
): (req: Request, res: express.Response) => Promise<void> {
return async (req: Request, res: express.Response) => {
try {
await handler(req, res);
} catch (err) {
logger.error("Unhandled error", err);
if (!res.headersSent) {
res.status(500).send("Internal Server Error");
}
}
};
}
3 changes: 2 additions & 1 deletion src/v1/providers/https.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
HttpsError,
onCallHandler,
Request,
withErrorHandler,
} from "../../common/providers/https";
import { HttpsFunction, optionsToEndpoint, optionsToTrigger, Runnable } from "../cloud-functions";
import { DeploymentOptions } from "../function-configuration";
Expand Down Expand Up @@ -66,7 +67,7 @@ export function _onRequestWithOptions(
): HttpsFunction {
// lets us add __endpoint without altering handler:
const cloudFunction: any = (req: Request, res: express.Response) => {
return wrapTraceContext(withInit(handler))(req, res);
return wrapTraceContext(withInit(withErrorHandler(handler)))(req, res);
};
cloudFunction.__trigger = {
...optionsToTrigger(options),
Expand Down
3 changes: 3 additions & 0 deletions src/v2/providers/https.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
onCallHandler,
Request,
AuthData,
withErrorHandler,
} from "../../common/providers/https";
import { initV2Endpoint, ManifestEndpoint } from "../../runtime/manifest";
import { GlobalOptions, SupportedRegion } from "../options";
Expand Down Expand Up @@ -319,6 +320,8 @@ export function onRequest(
opts = optsOrHandler as HttpsOptions;
}

handler = withErrorHandler(handler);

if (isDebugFeatureEnabled("enableCors") || "cors" in opts) {
let origin = opts.cors instanceof Expression ? opts.cors.value() : opts.cors;
if (isDebugFeatureEnabled("enableCors")) {
Expand Down
Loading