feat: Add error handling via ErrorModule (#20)

scottlovegrove · web-flow · commit e719edb7fec5 · 2022-08-24T11:05:03.000+01:00
diff --git a/src/modules/app.module.ts b/src/modules/app.module.ts
@@ -4,6 +4,7 @@ import {
     AuthenticationClient,
     AuthModule,
     CoreModule,
+    ErrorModule,
     LoginService,
     TokenValidator,
     UserDatabaseService as UserDatabaseServiceBase,
@@ -69,6 +70,11 @@ import { DatabaseModule } from './database.module'
             ],
             exports: [GoogleSheetsService, GoogleLoginService],
         }),
+        ErrorModule.forRoot({
+            errorCardOptions: {
+                helpCenterLink: Sheets.HELP_CENTER_LINK,
+            },
+        }),
     ],
     providers: [
         AdaptiveCardService,
diff --git a/src/services/actions.service.spec.ts b/src/services/actions.service.spec.ts
@@ -5,7 +5,8 @@ import { Test } from '@nestjs/testing'
 import { getRepositoryToken } from '@nestjs/typeorm'
 import MockDate from 'mockdate'
 
-import { buildTask, buildUser } from '../../test/fixtures'
+import { buildUser } from '../../test/fixtures'
+import { setupGetTasks, setupGetToken, setupGetUser } from '../../test/setups'
 import { CardActions as SheetCardActions } from '../constants/card-actions'
 import { User } from '../entities/user.entity'
 import * as csvHelpers from '../utils/csv-helpers'
@@ -131,6 +132,9 @@ describe('ActionsService', () => {
                         content: 'My Project',
                         contentPlain: 'My Project',
                     } as ContextMenuData,
+                    inputs: {
+                        'Input.completed': 'true',
+                    },
                 },
                 extensionType: 'context-menu',
                 maximumDoistCardVersion: 0.5,
@@ -293,21 +297,4 @@ describe('ActionsService', () => {
             })
         })
     })
-
-    function setupGetUser(user: User | undefined) {
-        const getUser = jest.spyOn(target['userDatabaseService'], 'getUser')
-        getUser.mockImplementation(() => Promise.resolve(user))
-    }
-
-    function setupGetToken(token: string | undefined) {
-        jest.spyOn(target['googleSheetsService'], 'getCurrentOrRefreshedToken').mockImplementation(
-            () => Promise.resolve(token ? { token, userId: '42' } : undefined),
-        )
-    }
-
-    function setupGetTasks() {
-        jest.spyOn(TodoistApi.prototype, 'getTasks').mockImplementation(() =>
-            Promise.resolve([buildTask()]),
-        )
-    }
 })
diff --git a/src/services/actions.service.ts b/src/services/actions.service.ts
@@ -1,14 +1,17 @@
 import { formatString } from '@doist/integrations-common'
-import { TodoistApi } from '@doist/todoist-api-typescript'
+import { Section, Task, TodoistApi } from '@doist/todoist-api-typescript'
 import {
     ActionsService as ActionsServiceBase,
     CardActions,
     DoistCardBridgeFactory,
+    IntegrationException,
+    isForbiddenError,
+    isUnauthorizedError,
     Submit,
     TranslationService,
 } from '@doist/ui-extensions-server'
 
-import { Injectable } from '@nestjs/common'
+import { BadRequestException, Injectable } from '@nestjs/common'
 
 import { getConfiguration } from '../config/configuration'
 import { CardActions as SheetsCardActions } from '../constants/card-actions'
@@ -97,48 +100,81 @@ export class ActionsService extends ActionsServiceBase {
             return this.logout(request)
         }
 
+        if (!action.inputs) {
+            throw new IntegrationException({
+                error: new BadRequestException(),
+            })
+        }
+
         const contextData = action.params as ContextMenuData
         const todoistClient = new TodoistApi(getConfiguration().todoistAuthToken)
 
         const exportOptions = getExportOptions(action.inputs)
 
-        const tasks = await todoistClient.getTasks({ projectId: contextData.sourceId })
+        let tasks: Task[] = []
+        let sections: Section[] = []
 
-        if (tasks.length === 0) {
-            return {
-                card: this.adaptiveCardsService.noTasksCard({
-                    projectName: contextData.content,
-                }),
+        try {
+            tasks = await todoistClient.getTasks({ projectId: contextData.sourceId })
+
+            if (tasks.length === 0) {
+                return {
+                    card: this.adaptiveCardsService.noTasksCard({
+                        projectName: contextData.content,
+                    }),
+                }
             }
+
+            // Only fetch sections if we're using them, otherwise it's a wasted call
+            sections = exportOptions['section']
+                ? await todoistClient.getSections(contextData.sourceId)
+                : []
+        } catch (error: unknown) {
+            throw new IntegrationException({
+                error,
+                overrides: {
+                    retryAction: action,
+                },
+            })
         }
 
-        // Only fetch sections if we're using them, otherwise it's a wasted call
-        const sections = exportOptions['section']
-            ? await todoistClient.getSections(contextData.sourceId)
-            : []
+        try {
+            const csvData = convertTasksToCsvString({
+                tasks,
+                sections,
+                exportOptions,
+            })
 
-        const csvData = convertTasksToCsvString({
-            tasks,
-            sections,
-            exportOptions,
-        })
+            const sheetUrl = await this.googleSheetsService.exportToSheets({
+                title: this.createSheetName(contextData.content),
+                csvData: csvData,
+                authToken: token.token,
+            })
 
-        const sheetUrl = await this.googleSheetsService.exportToSheets({
-            title: this.createSheetName(contextData.content),
-            csvData: csvData,
-            authToken: token.token,
-        })
+            return {
+                bridges: [
+                    DoistCardBridgeFactory.createNotificationBridge({
+                        text: this.translationService.getTranslation(Sheets.EXPORT_COMPLETED),
+                        type: 'success',
+                        actionText: this.translationService.getTranslation(Sheets.VIEW_SHEET),
+                        actionUrl: sheetUrl,
+                    }),
+                    DoistCardBridgeFactory.finished,
+                ],
+            }
+        } catch (error: unknown) {
+            if (isUnauthorizedError(error) || isForbiddenError(error)) {
+                // If we have become unauthorized, we need to treat this like a sign out
+                // so tokens need to be removed.
+                return this.logout(request)
+            }
 
-        return {
-            bridges: [
-                DoistCardBridgeFactory.createNotificationBridge({
-                    text: this.translationService.getTranslation(Sheets.EXPORT_COMPLETED),
-                    type: 'success',
-                    actionText: this.translationService.getTranslation(Sheets.VIEW_SHEET),
-                    actionUrl: sheetUrl,
-                }),
-                DoistCardBridgeFactory.finished,
-            ],
+            throw new IntegrationException({
+                error,
+                overrides: {
+                    retryAction: action,
+                },
+            })
         }
     }
 
diff --git a/test/e2e/export.e2e-spec.ts b/test/e2e/export.e2e-spec.ts
@@ -4,8 +4,8 @@ import request from 'supertest'
 
 import { CardActions as SheetCardActions } from '../../src/constants/card-actions'
 import { GoogleSheetsService } from '../../src/services/google-sheets.service'
-import { UserDatabaseService } from '../../src/services/user-database.service'
 import { buildUser } from '../fixtures'
+import { setupGetToken, setupGetUser } from '../setups'
 
 import { createTestApp } from './helpers'
 
@@ -23,12 +23,9 @@ describe('export e2e tests', () => {
     })
 
     it('returns the no tasks card if no tasks for the specified project', () => {
-        jest.spyOn(UserDatabaseService.prototype, 'getUser').mockImplementation(() =>
-            Promise.resolve(buildUser()),
-        )
-        jest.spyOn(GoogleSheetsService.prototype, 'getCurrentOrRefreshedToken').mockImplementation(
-            () => Promise.resolve({ token: 'token', userId: '42' }),
-        )
+        setupGetUser(buildUser())
+        setupGetToken('token')
+
         jest.spyOn(TodoistApi.prototype, 'getTasks').mockImplementation(() => Promise.resolve([]))
 
         return request(app.getHttpServer())
@@ -45,6 +42,9 @@ describe('export e2e tests', () => {
                         content: 'My Project',
                         contentPlain: 'My Project',
                     },
+                    inputs: {
+                        'Input.completed': 'true',
+                    },
                 },
             })
             .expect(200)
@@ -55,4 +55,114 @@ describe('export e2e tests', () => {
                 expect(JSON.stringify(body)).toMatch(/\*\*My Project\*\* has no tasks to export/)
             })
     })
+
+    it('returns the error card if inputs are not present (should not happen, but you never know)', () => {
+        setupGetUser(buildUser())
+        setupGetToken('token')
+
+        return request(app.getHttpServer())
+            .post('/process')
+            .send({
+                context: { user: { id: 42 }, theme: 'light' },
+                action: {
+                    actionType: 'submit',
+                    actionId: SheetCardActions.Export,
+                    params: {
+                        source: 'project',
+                        sourceId: 1234,
+                        url: 'https://google.com',
+                        content: 'My Project',
+                        contentPlain: 'My Project',
+                    },
+                },
+            })
+            .expect(400)
+            .expect('Content-Type', /json/)
+            .then((response) => {
+                const body = response.body as DoistCardResponse
+                expect(body.card).toBeDefined()
+
+                const json = JSON.stringify(body)
+                expect(json).toMatch(/Unfortunately, it looks like something went wrong./)
+                expect(json).not.toMatch(/Retry/)
+            })
+    })
+
+    it('returns the error card if talking to Todoist fails', () => {
+        setupGetUser(buildUser())
+        setupGetToken('token')
+
+        jest.spyOn(TodoistApi.prototype, 'getTasks').mockImplementation(() => {
+            throw new Error('Error talking to Todoist')
+        })
+
+        return request(app.getHttpServer())
+            .post('/process')
+            .send({
+                context: { user: { id: 42 }, theme: 'light' },
+                action: {
+                    actionType: 'submit',
+                    actionId: SheetCardActions.Export,
+                    params: {
+                        source: 'project',
+                        sourceId: 1234,
+                        url: 'https://google.com',
+                        content: 'My Project',
+                        contentPlain: 'My Project',
+                    },
+                    inputs: {
+                        'Input.completed': 'true',
+                    },
+                },
+            })
+            .expect(500)
+            .expect('Content-Type', /json/)
+            .then((response) => {
+                const body = response.body as DoistCardResponse
+                expect(body.card).toBeDefined()
+
+                const json = JSON.stringify(body)
+                expect(json).toMatch(/Unfortunately, it looks like something went wrong./)
+                expect(json).toMatch(/Retry/)
+            })
+    })
+
+    it('returns the error card if talking to Google fails', () => {
+        setupGetUser(buildUser())
+        setupGetToken('token')
+
+        jest.spyOn(GoogleSheetsService.prototype, 'exportToSheets').mockImplementation(() => {
+            throw new Error('Generic error talking to Google')
+        })
+
+        return request(app.getHttpServer())
+            .post('/process')
+            .send({
+                context: { user: { id: 42 }, theme: 'light' },
+                action: {
+                    actionType: 'submit',
+                    actionId: SheetCardActions.Export,
+                    params: {
+                        source: 'project',
+                        sourceId: 1234,
+                        url: 'https://google.com',
+                        content: 'My Project',
+                        contentPlain: 'My Project',
+                    },
+                    inputs: {
+                        'Input.completed': 'true',
+                    },
+                },
+            })
+            .expect(500)
+            .expect('Content-Type', /json/)
+            .then((response) => {
+                const body = response.body as DoistCardResponse
+                expect(body.card).toBeDefined()
+
+                const json = JSON.stringify(body)
+                expect(json).toMatch(/Unfortunately, it looks like something went wrong./)
+                expect(json).toMatch(/Retry/)
+            })
+    })
 })
diff --git a/test/setups.ts b/test/setups.ts
@@ -0,0 +1,25 @@
+import { TodoistApi } from '@doist/todoist-api-typescript'
+
+import { GoogleSheetsService } from '../src/services/google-sheets.service'
+import { UserDatabaseService } from '../src/services/user-database.service'
+
+import { buildTask } from './fixtures'
+
+import type { User } from '../src/entities/user.entity'
+
+export function setupGetUser(user: User | undefined) {
+    const getUser = jest.spyOn(UserDatabaseService.prototype, 'getUser')
+    getUser.mockImplementation(() => Promise.resolve(user))
+}
+
+export function setupGetToken(token: string | undefined) {
+    jest.spyOn(GoogleSheetsService.prototype, 'getCurrentOrRefreshedToken').mockImplementation(() =>
+        Promise.resolve(token ? { token, userId: '42' } : undefined),
+    )
+}
+
+export function setupGetTasks() {
+    jest.spyOn(TodoistApi.prototype, 'getTasks').mockImplementation(() =>
+        Promise.resolve([buildTask()]),
+    )
+}
