From ea65d37474d18e9552c0f8f72317c041c225f738 Mon Sep 17 00:00:00 2001 From: Lukasz Ostrowski Date: Tue, 14 Feb 2023 16:19:07 +0100 Subject: [PATCH] Add better error handling for async webhook factory (#176) * Add better error handling for async webhook factory * Add docs, make formatter function async * Add docs, make formatter function async --- .eslintrc | 3 +- docs/saleor-async-webhook.md | 52 +++++++++++++--- .../next/saleor-async-webhook.test.ts | 61 +++++++++++++++++++ src/handlers/next/saleor-async-webhook.ts | 50 ++++++++++++++- 4 files changed, 156 insertions(+), 10 deletions(-) diff --git a/.eslintrc b/.eslintrc index c54fd77..50e3a37 100644 --- a/.eslintrc +++ b/.eslintrc @@ -57,7 +57,8 @@ "import/no-cycle": "off", // pathpidia issue "import/prefer-default-export": "off", "@typescript-eslint/no-misused-promises": ["error"], - "@typescript-eslint/no-floating-promises": ["error"] + "@typescript-eslint/no-floating-promises": ["error"], + "class-methods-use-this": "off" }, "settings": { "import/parsers": { diff --git a/docs/saleor-async-webhook.md b/docs/saleor-async-webhook.md index 2e54df0..654655e 100644 --- a/docs/saleor-async-webhook.md +++ b/docs/saleor-async-webhook.md @@ -49,7 +49,41 @@ export const orderCreatedWebhook = new SaleorAsyncWebhook({ /** * Subscription query, telling Saleor what payload app expects */ - query: "TODO", + query: ` + subscription { + event { + ... on OrderCreated { + order { + id + } + } + } + } + `, + /** + * Optional + * + * Read internal errors + */ + onError(error: WebhookError | Error) { + // Can be used to e.g. trace errors + sentry.captureError(error); + }, + /** + * Optional + * Allows to set custom error response. If not provided, default mapping and message will be responsed + * if Webhook validation fails + */ + async formatErrorResponse( + error: WebhookError | Error, + req: NextApiRequest, + res: NextApiResponse + ) { + return { + code: 400, + body: "My custom response", + }; + }, }); ``` @@ -125,13 +159,16 @@ export const ExampleProductUpdatedSubscription = gql` ${ProductUpdatedWebhookPayload} subscription ExampleProductUpdated { event { - fragment ProductUpdatedWebhookPayload on ProductUpdated { - product { - id - name + fragment + ProductUpdatedWebhookPayload + on + ProductUpdated { + product { + id + name + } } } - } } `; @@ -141,4 +178,5 @@ export const productUpdatedWebhook = new SaleorAsyncWebhook { // Check if test handler was used by the wrapper expect(testHandler).toBeCalledTimes(1); }); + + it("Calls callbacks for error handling", async () => { + const onErrorCallback = vi.fn(); + const formatErrorCallback = vi.fn().mockImplementation(async () => ({ + code: 401, + body: "My Body", + })); + + const webhook = new SaleorAsyncWebhook({ + ...validAsyncWebhookConfiguration, + onError: onErrorCallback, + formatErrorResponse: formatErrorCallback, + }); + + // prepare mocked context returned by mocked process function + vi.mock("./process-async-saleor-webhook"); + + vi.mocked(processAsyncSaleorWebhook).mockImplementationOnce(async () => { + /** + * This mock should throw WebhookError, but there was TypeError related to constructor of extended class. + * Try "throw new WebhookError()" to check it. + * + * For test suite it doesn't matter, because errors thrown from source code are valid + */ + throw new Error("Test error message"); + }); + + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const testHandler: NextWebhookApiHandler = vi.fn().mockImplementation((req, res, context) => { + if (context.payload.data === "test_payload") { + res.status(200).end(); + return; + } + throw new Error("Test payload has not been passed to handler function"); + }); + + const { req, res } = createMocks(); + const wrappedHandler = webhook.createHandler(testHandler); + + await wrappedHandler(req, res); + + /** + * Response should match formatErrorCallback + */ + expect(res.statusCode).toBe(401); + expect(res._getData()).toBe("My Body"); + /** + * TODO This assertion fails, due to WebhookError constructor: + * [TypeError: Class constructor WebhookError cannot be invoked without 'new'] + */ + expect(onErrorCallback).toHaveBeenCalledWith( + expect.objectContaining({ + message: "Test error message", + }) + ); + + /** + * Handler should not be called, since it thrown before + */ + expect(testHandler).not.toHaveBeenCalled(); + }); }); diff --git a/src/handlers/next/saleor-async-webhook.ts b/src/handlers/next/saleor-async-webhook.ts index 3781dc1..4d81ffe 100644 --- a/src/handlers/next/saleor-async-webhook.ts +++ b/src/handlers/next/saleor-async-webhook.ts @@ -20,6 +20,15 @@ interface WebhookManifestConfigurationBase { asyncEvent: AsyncWebhookEventType; isActive?: boolean; apl: APL; + onError?(error: WebhookError | Error): void; + formatErrorResponse?( + error: WebhookError | Error, + req: NextApiRequest, + res: NextApiResponse + ): Promise<{ + code: number; + body: object | string; + }>; } interface WebhookManifestConfigurationWithAst extends WebhookManifestConfigurationBase { @@ -72,6 +81,10 @@ export class SaleorAsyncWebhook { apl: APL; + onError: WebhookManifestConfigurationBase["onError"]; + + formatErrorResponse: WebhookManifestConfigurationBase["formatErrorResponse"]; + constructor(configuration: WebhookManifestConfiguration) { const { name, webhookPath, asyncEvent, apl, isActive = true } = configuration; this.name = name || `${asyncEvent} webhook`; @@ -92,6 +105,8 @@ export class SaleorAsyncWebhook { this.asyncEvent = asyncEvent; this.isActive = isActive; this.apl = apl; + this.onError = configuration.onError; + this.formatErrorResponse = configuration.formatErrorResponse; } /** @@ -141,15 +156,46 @@ export class SaleorAsyncWebhook { debug("Incoming request validated. Call handlerFn"); return handlerFn(req, res, context); }) - .catch((e) => { + .catch(async (e) => { debug(`Unexpected error during processing the webhook ${this.name}`); if (e instanceof WebhookError) { debug(`Validation error: ${e.message}`); - res.status(AsyncWebhookErrorCodeMap[e.errorType] || 400).end(); + + if (this.onError) { + this.onError(e); + } + + if (this.formatErrorResponse) { + const { code, body } = await this.formatErrorResponse(e, req, res); + + res.status(code).send(body); + + return; + } + + res.status(AsyncWebhookErrorCodeMap[e.errorType] || 400).send({ + error: { + type: e.errorType, + message: e.message, + }, + }); return; } debug("Unexpected error: %O", e); + + if (this.onError) { + this.onError(e); + } + + if (this.formatErrorResponse) { + const { code, body } = await this.formatErrorResponse(e, req, res); + + res.status(code).send(body); + + return; + } + res.status(500).end(); }); };