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
This commit is contained in:
parent
652833146f
commit
ea65d37474
4 changed files with 156 additions and 10 deletions
|
@ -57,7 +57,8 @@
|
||||||
"import/no-cycle": "off", // pathpidia issue
|
"import/no-cycle": "off", // pathpidia issue
|
||||||
"import/prefer-default-export": "off",
|
"import/prefer-default-export": "off",
|
||||||
"@typescript-eslint/no-misused-promises": ["error"],
|
"@typescript-eslint/no-misused-promises": ["error"],
|
||||||
"@typescript-eslint/no-floating-promises": ["error"]
|
"@typescript-eslint/no-floating-promises": ["error"],
|
||||||
|
"class-methods-use-this": "off"
|
||||||
},
|
},
|
||||||
"settings": {
|
"settings": {
|
||||||
"import/parsers": {
|
"import/parsers": {
|
||||||
|
|
|
@ -49,7 +49,41 @@ export const orderCreatedWebhook = new SaleorAsyncWebhook<OrderPayload>({
|
||||||
/**
|
/**
|
||||||
* Subscription query, telling Saleor what payload app expects
|
* 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,7 +159,10 @@ export const ExampleProductUpdatedSubscription = gql`
|
||||||
${ProductUpdatedWebhookPayload}
|
${ProductUpdatedWebhookPayload}
|
||||||
subscription ExampleProductUpdated {
|
subscription ExampleProductUpdated {
|
||||||
event {
|
event {
|
||||||
fragment ProductUpdatedWebhookPayload on ProductUpdated {
|
fragment
|
||||||
|
ProductUpdatedWebhookPayload
|
||||||
|
on
|
||||||
|
ProductUpdated {
|
||||||
product {
|
product {
|
||||||
id
|
id
|
||||||
name
|
name
|
||||||
|
@ -142,3 +179,4 @@ export const productUpdatedWebhook = new SaleorAsyncWebhook<ProductUpdatedWebhoo
|
||||||
apl: saleorApp.apl,
|
apl: saleorApp.apl,
|
||||||
subscriptionQueryAst: ExampleProductUpdatedSubscription,
|
subscriptionQueryAst: ExampleProductUpdatedSubscription,
|
||||||
});
|
});
|
||||||
|
```
|
||||||
|
|
|
@ -114,4 +114,65 @@ describe("SaleorAsyncWebhook", () => {
|
||||||
// Check if test handler was used by the wrapper
|
// Check if test handler was used by the wrapper
|
||||||
expect(testHandler).toBeCalledTimes(1);
|
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();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
@ -20,6 +20,15 @@ interface WebhookManifestConfigurationBase {
|
||||||
asyncEvent: AsyncWebhookEventType;
|
asyncEvent: AsyncWebhookEventType;
|
||||||
isActive?: boolean;
|
isActive?: boolean;
|
||||||
apl: APL;
|
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 {
|
interface WebhookManifestConfigurationWithAst extends WebhookManifestConfigurationBase {
|
||||||
|
@ -72,6 +81,10 @@ export class SaleorAsyncWebhook<TPayload = unknown> {
|
||||||
|
|
||||||
apl: APL;
|
apl: APL;
|
||||||
|
|
||||||
|
onError: WebhookManifestConfigurationBase["onError"];
|
||||||
|
|
||||||
|
formatErrorResponse: WebhookManifestConfigurationBase["formatErrorResponse"];
|
||||||
|
|
||||||
constructor(configuration: WebhookManifestConfiguration) {
|
constructor(configuration: WebhookManifestConfiguration) {
|
||||||
const { name, webhookPath, asyncEvent, apl, isActive = true } = configuration;
|
const { name, webhookPath, asyncEvent, apl, isActive = true } = configuration;
|
||||||
this.name = name || `${asyncEvent} webhook`;
|
this.name = name || `${asyncEvent} webhook`;
|
||||||
|
@ -92,6 +105,8 @@ export class SaleorAsyncWebhook<TPayload = unknown> {
|
||||||
this.asyncEvent = asyncEvent;
|
this.asyncEvent = asyncEvent;
|
||||||
this.isActive = isActive;
|
this.isActive = isActive;
|
||||||
this.apl = apl;
|
this.apl = apl;
|
||||||
|
this.onError = configuration.onError;
|
||||||
|
this.formatErrorResponse = configuration.formatErrorResponse;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -141,15 +156,46 @@ export class SaleorAsyncWebhook<TPayload = unknown> {
|
||||||
debug("Incoming request validated. Call handlerFn");
|
debug("Incoming request validated. Call handlerFn");
|
||||||
return handlerFn(req, res, context);
|
return handlerFn(req, res, context);
|
||||||
})
|
})
|
||||||
.catch((e) => {
|
.catch(async (e) => {
|
||||||
debug(`Unexpected error during processing the webhook ${this.name}`);
|
debug(`Unexpected error during processing the webhook ${this.name}`);
|
||||||
|
|
||||||
if (e instanceof WebhookError) {
|
if (e instanceof WebhookError) {
|
||||||
debug(`Validation error: ${e.message}`);
|
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;
|
return;
|
||||||
}
|
}
|
||||||
debug("Unexpected error: %O", e);
|
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();
|
res.status(500).end();
|
||||||
});
|
});
|
||||||
};
|
};
|
||||||
|
|
Loading…
Reference in a new issue