From 70cb93260eac853cbf4469aea4668f61c603bd4a Mon Sep 17 00:00:00 2001 From: Michael Bromley Date: Fri, 11 Dec 2020 11:16:42 +0100 Subject: [PATCH] feat(email-plugin): Improve error logging Closes #574 --- packages/email-plugin/src/email-processor.ts | 32 ++++++--- packages/email-plugin/src/event-handler.ts | 20 ++++-- packages/email-plugin/src/plugin.spec.ts | 72 +++++++++++++++++++ packages/email-plugin/src/types.ts | 2 +- .../test-templates/bad-template/body.hbs | 5 ++ 5 files changed, 114 insertions(+), 17 deletions(-) create mode 100644 packages/email-plugin/test-templates/bad-template/body.hbs diff --git a/packages/email-plugin/src/email-processor.ts b/packages/email-plugin/src/email-processor.ts index 6d9e4975de..ccacc0ff91 100644 --- a/packages/email-plugin/src/email-processor.ts +++ b/packages/email-plugin/src/email-processor.ts @@ -1,7 +1,8 @@ -import { InternalServerError } from '@vendure/core'; +import { InternalServerError, Logger } from '@vendure/core'; import fs from 'fs-extra'; import { isDevModeOptions } from './common'; +import { loggerCtx } from './constants'; import { EmailSender } from './email-sender'; import { HandlebarsMjmlGenerator } from './handlebars-mjml-generator'; import { TemplateLoader } from './template-loader'; @@ -50,15 +51,24 @@ export class EmailProcessor { } async process(data: IntermediateEmailDetails) { - const bodySource = await this.templateLoader.loadTemplate(data.type, data.templateFile); - const generated = await this.generator.generate( - data.from, - data.subject, - bodySource, - data.templateVars, - ); - const emailDetails = { ...generated, recipient: data.recipient }; - await this.emailSender.send(emailDetails, this.transport); - return true; + try { + const bodySource = await this.templateLoader.loadTemplate(data.type, data.templateFile); + const generated = await this.generator.generate( + data.from, + data.subject, + bodySource, + data.templateVars, + ); + const emailDetails = { ...generated, recipient: data.recipient }; + await this.emailSender.send(emailDetails, this.transport); + return true; + } catch (err: unknown) { + if (err instanceof Error) { + Logger.error(err.message, loggerCtx, err.stack); + } else { + Logger.error(String(err), loggerCtx); + } + return false; + } } } diff --git a/packages/email-plugin/src/event-handler.ts b/packages/email-plugin/src/event-handler.ts index a9416a34aa..5a6d2d798a 100644 --- a/packages/email-plugin/src/event-handler.ts +++ b/packages/email-plugin/src/event-handler.ts @@ -1,7 +1,8 @@ import { LanguageCode } from '@vendure/common/lib/generated-types'; import { Type } from '@vendure/common/lib/shared-types'; -import { Injector } from '@vendure/core'; +import { Injector, Logger } from '@vendure/core'; +import { loggerCtx } from './constants'; import { EmailEventListener, EmailTemplateConfig, SetTemplateVarsFn } from './event-listener'; import { EventWithAsyncData, EventWithContext, IntermediateEmailDetails, LoadDataFn } from './types'; @@ -178,10 +179,19 @@ export class EmailEventHandler).data = await this._loadDataFn({ - event, - injector, - }); + try { + (event as EventWithAsyncData).data = await this._loadDataFn({ + event, + injector, + }); + } catch (err: unknown) { + if (err instanceof Error) { + Logger.error(err.message, loggerCtx, err.stack); + } else { + Logger.error(String(err), loggerCtx); + } + return; + } } if (!this.setRecipientFn) { throw new Error( diff --git a/packages/email-plugin/src/plugin.spec.ts b/packages/email-plugin/src/plugin.spec.ts index c07905791a..dbc6bedfda 100644 --- a/packages/email-plugin/src/plugin.spec.ts +++ b/packages/email-plugin/src/plugin.spec.ts @@ -5,6 +5,7 @@ import { DEFAULT_CHANNEL_CODE } from '@vendure/common/lib/shared-constants'; import { EventBus, LanguageCode, + Logger, Order, OrderStateTransitionEvent, PluginCommonModule, @@ -12,6 +13,7 @@ import { RequestContext, VendureEvent, } from '@vendure/core'; +import { TestingLogger } from '@vendure/testing'; import path from 'path'; import { orderConfirmationHandler } from './default-email-handlers'; @@ -25,6 +27,8 @@ describe('EmailPlugin', () => { let onSend: jest.Mock; let module: TestingModule; + const testingLogger = new TestingLogger(() => jest.fn()); + async function initPluginWithHandlers( handlers: Array>, options?: Partial, @@ -51,6 +55,9 @@ describe('EmailPlugin', () => { providers: [MockService], }).compile(); + Logger.useLogger(testingLogger); + module.useLogger(new Logger()); + const plugin = module.get(EmailPlugin); eventBus = module.get(EventBus); await plugin.onVendureBootstrap(); @@ -492,6 +499,71 @@ describe('EmailPlugin', () => { expect(onSend.mock.calls[0][0].subject).toBe(`Order confirmation for #${order.code}`); }); }); + + describe('error handling', () => { + it('Logs an error if the template file is not found', async () => { + const ctx = RequestContext.deserialize({ + _channel: { code: DEFAULT_CHANNEL_CODE }, + _languageCode: LanguageCode.en, + } as any); + testingLogger.errorSpy.mockClear(); + + const handler = new EmailEventListener('no-template') + .on(MockEvent) + .setFrom('"test from" ') + .setRecipient(() => 'test@test.com') + .setSubject('Hello {{ subjectVar }}'); + + await initPluginWithHandlers([handler]); + + eventBus.publish(new MockEvent(ctx, true)); + await pause(); + expect(testingLogger.errorSpy.mock.calls[0][0]).toContain(`ENOENT: no such file or directory`); + }); + + it('Logs a Handlebars error if the template is invalid', async () => { + const ctx = RequestContext.deserialize({ + _channel: { code: DEFAULT_CHANNEL_CODE }, + _languageCode: LanguageCode.en, + } as any); + testingLogger.errorSpy.mockClear(); + + const handler = new EmailEventListener('bad-template') + .on(MockEvent) + .setFrom('"test from" ') + .setRecipient(() => 'test@test.com') + .setSubject('Hello {{ subjectVar }}'); + + await initPluginWithHandlers([handler]); + + eventBus.publish(new MockEvent(ctx, true)); + await pause(); + expect(testingLogger.errorSpy.mock.calls[0][0]).toContain(`Parse error on line 3:`); + }); + + it('Logs an error if the loadData method throws', async () => { + const ctx = RequestContext.deserialize({ + _channel: { code: DEFAULT_CHANNEL_CODE }, + _languageCode: LanguageCode.en, + } as any); + testingLogger.errorSpy.mockClear(); + + const handler = new EmailEventListener('bad-template') + .on(MockEvent) + .setFrom('"test from" ') + .setRecipient(() => 'test@test.com') + .setSubject('Hello {{ subjectVar }}') + .loadData(context => { + throw new Error('something went horribly wrong!'); + }); + + await initPluginWithHandlers([handler]); + + eventBus.publish(new MockEvent(ctx, true)); + await pause(); + expect(testingLogger.errorSpy.mock.calls[0][0]).toContain(`something went horribly wrong!`); + }); + }); }); const pause = () => new Promise(resolve => setTimeout(resolve, 100)); diff --git a/packages/email-plugin/src/types.ts b/packages/email-plugin/src/types.ts index dad3559cd9..f82509d0a2 100644 --- a/packages/email-plugin/src/types.ts +++ b/packages/email-plugin/src/types.ts @@ -265,7 +265,7 @@ export interface TestingTransportOptions { export interface EmailGenerator { /** * @description - * Any neccesary setup can be performed here. + * Any necessary setup can be performed here. */ onInit?(options: EmailPluginOptions): void | Promise; diff --git a/packages/email-plugin/test-templates/bad-template/body.hbs b/packages/email-plugin/test-templates/bad-template/body.hbs new file mode 100644 index 0000000000..432ab6b89d --- /dev/null +++ b/packages/email-plugin/test-templates/bad-template/body.hbs @@ -0,0 +1,5 @@ +{{ this is a bad Handlbars template }} + +{{ < designed }} + +{{ to produce an error!! }}}