diff --git a/src/app-gocardless/app-gocardless.js b/src/app-gocardless/app-gocardless.js index fd6fbc3e9b..85cc249202 100644 --- a/src/app-gocardless/app-gocardless.js +++ b/src/app-gocardless/app-gocardless.js @@ -5,9 +5,10 @@ import { inspect } from 'util'; import { goCardlessService } from './services/gocardless-service.js'; import { - RequisitionNotLinked, AccountNotLinedToRequisition, GenericGoCardlessError, + RateLimitError, + RequisitionNotLinked, } from './errors.js'; import { handleError } from './util/handle-error.js'; import { sha256String } from '../util/hash.js'; @@ -191,6 +192,14 @@ app.post( reason: 'Account not linked with this requisition', }); break; + case error instanceof RateLimitError: + sendErrorResponse({ + error_type: 'RATE_LIMIT_EXCEEDED', + error_code: 'NORDIGEN_ERROR', + status: 'rejected', + reason: 'Rate limit exceeded', + }); + break; case error instanceof GenericGoCardlessError: console.log('Something went wrong', inspect(error, { depth: null })); sendErrorResponse({ diff --git a/src/app-gocardless/services/gocardless-service.js b/src/app-gocardless/services/gocardless-service.js index 600fc8ac71..61efd6eeec 100644 --- a/src/app-gocardless/services/gocardless-service.js +++ b/src/app-gocardless/services/gocardless-service.js @@ -1,15 +1,16 @@ import BankFactory, { BANKS_WITH_LIMITED_HISTORY } from '../bank-factory.js'; import { - RequisitionNotLinked, + AccessDeniedError, AccountNotLinedToRequisition, + GenericGoCardlessError, InvalidInputDataError, InvalidGoCardlessTokenError, - AccessDeniedError, NotFoundError, - ResourceSuspended, RateLimitError, - UnknownError, + ResourceSuspended, + RequisitionNotLinked, ServiceError, + UnknownError, } from '../errors.js'; import * as nordigenNode from 'nordigen-node'; import * as uuid from 'uuid'; @@ -35,26 +36,28 @@ const getGocardlessClient = () => { return clients.get(hash); }; -export const handleGoCardlessError = (response) => { - switch (response.status_code) { +export const handleGoCardlessError = (error) => { + const status = error?.response?.status; + + switch (status) { case 400: - throw new InvalidInputDataError(response); + throw new InvalidInputDataError(error); case 401: - throw new InvalidGoCardlessTokenError(response); + throw new InvalidGoCardlessTokenError(error); case 403: - throw new AccessDeniedError(response); + throw new AccessDeniedError(error); case 404: - throw new NotFoundError(response); + throw new NotFoundError(error); case 409: - throw new ResourceSuspended(response); + throw new ResourceSuspended(error); case 429: - throw new RateLimitError(response); + throw new RateLimitError(error); case 500: - throw new UnknownError(response); + throw new UnknownError(error); case 503: - throw new ServiceError(response); + throw new ServiceError(error); default: - return; + throw new GenericGoCardlessError(error); } }; @@ -87,8 +90,11 @@ export const goCardlessService = { if (isExpiredJwtToken(getGocardlessClient().token)) { // Generate new access token. Token is valid for 24 hours // Note: access_token is automatically injected to other requests after you successfully obtain it - const tokenData = await client.generateToken(); - handleGoCardlessError(tokenData); + try { + await client.generateToken(); + } catch (error) { + handleGoCardlessError(error); + } } }, @@ -261,23 +267,26 @@ export const goCardlessService = { const institution = await goCardlessService.getInstitution(institutionId); const bank = BankFactory(institutionId); - const response = await client.initSession({ - redirectUrl: host + '/gocardless/link', - institutionId, - referenceId: uuid.v4(), - accessValidForDays: bank.accessValidForDays, - maxHistoricalDays: BANKS_WITH_LIMITED_HISTORY.includes(institutionId) - ? Number(institution.transaction_total_days) >= 90 - ? '89' - : institution.transaction_total_days - : institution.transaction_total_days, - userLanguage: 'en', - ssn: null, - redirectImmediate: false, - accountSelection: false, - }); - - handleGoCardlessError(response); + let response; + try { + response = await client.initSession({ + redirectUrl: host + '/gocardless/link', + institutionId, + referenceId: uuid.v4(), + accessValidForDays: bank.accessValidForDays, + maxHistoricalDays: BANKS_WITH_LIMITED_HISTORY.includes(institutionId) + ? Number(institution.transaction_total_days) >= 90 + ? '89' + : institution.transaction_total_days + : institution.transaction_total_days, + userLanguage: 'en', + ssn: null, + redirectImmediate: false, + accountSelection: false, + }); + } catch (error) { + handleGoCardlessError(error); + } const { link, id: requisitionId } = response; @@ -302,9 +311,14 @@ export const goCardlessService = { */ deleteRequisition: async (requisitionId) => { await goCardlessService.getRequisition(requisitionId); - const response = client.deleteRequisition(requisitionId); - handleGoCardlessError(response); + let response; + try { + response = client.deleteRequisition(requisitionId); + } catch (error) { + handleGoCardlessError(error); + } + return response; }, @@ -325,9 +339,12 @@ export const goCardlessService = { getRequisition: async (requisitionId) => { await goCardlessService.setToken(); - const response = client.getRequisitionById(requisitionId); - - handleGoCardlessError(response); + let response; + try { + response = client.getRequisitionById(requisitionId); + } catch (error) { + handleGoCardlessError(error); + } return response; }, @@ -338,13 +355,15 @@ export const goCardlessService = { * @returns {Promise} */ getDetailedAccount: async (accountId) => { - const [detailedAccount, metadataAccount] = await Promise.all([ - client.getDetails(accountId), - client.getMetadata(accountId), - ]); - - handleGoCardlessError(detailedAccount); - handleGoCardlessError(metadataAccount); + let detailedAccount, metadataAccount; + try { + [detailedAccount, metadataAccount] = await Promise.all([ + client.getDetails(accountId), + client.getMetadata(accountId), + ]); + } catch (error) { + handleGoCardlessError(error); + } return { ...detailedAccount.account, @@ -361,9 +380,12 @@ export const goCardlessService = { * @returns {Promise} */ getAccountMetadata: async (accountId) => { - const response = await client.getMetadata(accountId); - - handleGoCardlessError(response); + let response; + try { + response = await client.getMetadata(accountId); + } catch (error) { + handleGoCardlessError(error); + } return response; }, @@ -382,9 +404,12 @@ export const goCardlessService = { * @returns {Promise>} */ getInstitutions: async (country) => { - const response = await client.getInstitutions(country); - - handleGoCardlessError(response); + let response; + try { + response = await client.getInstitutions(country); + } catch (error) { + handleGoCardlessError(error); + } return response; }, @@ -403,9 +428,12 @@ export const goCardlessService = { * @returns {Promise} */ getInstitution: async (institutionId) => { - const response = await client.getInstitutionById(institutionId); - - handleGoCardlessError(response); + let response; + try { + response = await client.getInstitutionById(institutionId); + } catch (error) { + handleGoCardlessError(error); + } return response; }, @@ -444,13 +472,16 @@ export const goCardlessService = { * @returns {Promise} */ getTransactions: async ({ institutionId, accountId, startDate, endDate }) => { - const response = await client.getTransactions({ - accountId, - dateFrom: startDate, - dateTo: endDate, - }); - - handleGoCardlessError(response); + let response; + try { + response = await client.getTransactions({ + accountId, + dateFrom: startDate, + dateTo: endDate, + }); + } catch (error) { + handleGoCardlessError(error); + } const bank = BankFactory(institutionId); response.transactions.booked = response.transactions.booked @@ -477,9 +508,12 @@ export const goCardlessService = { * @returns {Promise} */ getBalances: async (accountId) => { - const response = await client.getBalances(accountId); - - handleGoCardlessError(response); + let response; + try { + response = await client.getBalances(accountId); + } catch (error) { + handleGoCardlessError(error); + } return response; }, diff --git a/src/app-gocardless/services/tests/gocardless-service.spec.js b/src/app-gocardless/services/tests/gocardless-service.spec.js index ed7b1b0631..043ab5057c 100644 --- a/src/app-gocardless/services/tests/gocardless-service.spec.js +++ b/src/app-gocardless/services/tests/gocardless-service.spec.js @@ -1,20 +1,20 @@ import { jest } from '@jest/globals'; import { + AccessDeniedError, + AccountNotLinedToRequisition, + GenericGoCardlessError, InvalidInputDataError, InvalidGoCardlessTokenError, - AccessDeniedError, NotFoundError, - ResourceSuspended, RateLimitError, - UnknownError, - ServiceError, + ResourceSuspended, RequisitionNotLinked, - AccountNotLinedToRequisition, + ServiceError, + UnknownError, } from '../../errors.js'; import { mockedBalances, - mockUnknownError, mockTransactions, mockDetailedAccount, mockInstitution, @@ -252,17 +252,6 @@ describe('goCardlessService', () => { expect(createRequisitionSpy).toBeCalledTimes(1); }); - - it('handle error if status_code present in the response', async () => { - setTokenSpy.mockResolvedValue(); - getInstitutionSpy.mockResolvedValue(mockInstitution); - - createRequisitionSpy.mockResolvedValue(mockUnknownError); - - await expect(() => - goCardlessService.createRequisition(params), - ).rejects.toThrow(UnknownError); - }); }); describe('#deleteRequisition', () => { @@ -281,17 +270,6 @@ describe('goCardlessService', () => { expect(getRequisitionsSpy).toBeCalledTimes(1); expect(deleteRequisitionsSpy).toBeCalledTimes(1); }); - - it('handle error if status_code present in the response', async () => { - setTokenSpy.mockResolvedValue(); - - getRequisitionsSpy.mockResolvedValue(mockRequisition); - deleteRequisitionsSpy.mockReturnValue(mockUnknownError); - - await expect(() => - goCardlessService.deleteRequisition(requisitionId), - ).rejects.toThrow(UnknownError); - }); }); describe('#getRequisition', () => { @@ -308,16 +286,6 @@ describe('goCardlessService', () => { expect(setTokenSpy).toBeCalledTimes(1); expect(getRequisitionsSpy).toBeCalledTimes(1); }); - - it('handle error if status_code present in the response', async () => { - setTokenSpy.mockResolvedValue(); - - getRequisitionsSpy.mockReturnValue(mockUnknownError); - - await expect(() => - goCardlessService.getRequisition(requisitionId), - ).rejects.toThrow(UnknownError); - }); }); describe('#getDetailedAccount', () => { @@ -332,30 +300,6 @@ describe('goCardlessService', () => { expect(getDetailsSpy).toBeCalledTimes(1); expect(getMetadataSpy).toBeCalledTimes(1); }); - - it('handle error if status_code present in the detailedAccount response', async () => { - getDetailsSpy.mockResolvedValue(mockUnknownError); - getMetadataSpy.mockResolvedValue(mockAccountMetaData); - - await expect(() => - goCardlessService.getDetailedAccount(accountId), - ).rejects.toThrow(UnknownError); - - expect(getDetailsSpy).toBeCalledTimes(1); - expect(getMetadataSpy).toBeCalledTimes(1); - }); - - it('handle error if status_code present in the metadataAccount response', async () => { - getDetailsSpy.mockResolvedValue(mockAccountDetails); - getMetadataSpy.mockResolvedValue(mockUnknownError); - - await expect(() => - goCardlessService.getDetailedAccount(accountId), - ).rejects.toThrow(UnknownError); - - expect(getDetailsSpy).toBeCalledTimes(1); - expect(getMetadataSpy).toBeCalledTimes(1); - }); }); describe('#getInstitutions', () => { @@ -368,14 +312,6 @@ describe('goCardlessService', () => { ]); expect(getInstitutionsSpy).toBeCalledTimes(1); }); - - it('handle error if status_code present in the response', async () => { - getInstitutionsSpy.mockResolvedValue(mockUnknownError); - - await expect(() => - goCardlessService.getInstitutions({ country }), - ).rejects.toThrow(UnknownError); - }); }); describe('#getInstitution', () => { @@ -388,14 +324,6 @@ describe('goCardlessService', () => { ); expect(getInstitutionSpy).toBeCalledTimes(1); }); - - it('handle error if status_code present in the response', async () => { - getInstitutionSpy.mockResolvedValue(mockUnknownError); - - await expect(() => - goCardlessService.getInstitution(institutionId), - ).rejects.toThrow(UnknownError); - }); }); describe('#extendAccountsAboutInstitutions', () => { @@ -532,19 +460,6 @@ describe('goCardlessService', () => { `); expect(getTransactionsSpy).toBeCalledTimes(1); }); - - it('handle error if status_code present in the response', async () => { - getTransactionsSpy.mockResolvedValue(mockUnknownError); - - await expect(() => - goCardlessService.getTransactions({ - institutionId: 'SANDBOXFINANCE_SFIN0000', - accountId, - startDate: '', - endDate: '', - }), - ).rejects.toThrow(UnknownError); - }); }); describe('#getBalances', () => { @@ -556,69 +471,58 @@ describe('goCardlessService', () => { ); expect(getBalancesSpy).toBeCalledTimes(1); }); - - it('handle error if status_code present in the response', async () => { - getBalancesSpy.mockResolvedValue(mockUnknownError); - - await expect(() => - goCardlessService.getBalances(accountId), - ).rejects.toThrow(UnknownError); - }); }); }); describe('#handleGoCardlessError', () => { it('throws InvalidInputDataError for status code 400', () => { - const response = { status_code: 400 }; + const response = { response: { status: 400 } }; expect(() => handleGoCardlessError(response)).toThrow( InvalidInputDataError, ); }); it('throws InvalidGoCardlessTokenError for status code 401', () => { - const response = { status_code: 401 }; + const response = { response: { status: 401 } }; expect(() => handleGoCardlessError(response)).toThrow( InvalidGoCardlessTokenError, ); }); it('throws AccessDeniedError for status code 403', () => { - const response = { status_code: 403 }; + const response = { response: { status: 403 } }; expect(() => handleGoCardlessError(response)).toThrow(AccessDeniedError); }); it('throws NotFoundError for status code 404', () => { - const response = { status_code: 404 }; + const response = { response: { status: 404 } }; expect(() => handleGoCardlessError(response)).toThrow(NotFoundError); }); it('throws ResourceSuspended for status code 409', () => { - const response = { status_code: 409 }; + const response = { response: { status: 409 } }; expect(() => handleGoCardlessError(response)).toThrow(ResourceSuspended); }); it('throws RateLimitError for status code 429', () => { - const response = { status_code: 429 }; + const response = { response: { status: 429 } }; expect(() => handleGoCardlessError(response)).toThrow(RateLimitError); }); it('throws UnknownError for status code 500', () => { - const response = { status_code: 500 }; + const response = { response: { status: 500 } }; expect(() => handleGoCardlessError(response)).toThrow(UnknownError); }); it('throws ServiceError for status code 503', () => { - const response = { status_code: 503 }; + const response = { response: { status: 503 } }; expect(() => handleGoCardlessError(response)).toThrow(ServiceError); }); - it('does not throw an error for status code 200', () => { - const response = { status_code: 200 }; - expect(() => handleGoCardlessError(response)).not.toThrow(); - }); - - it('does not throw an error when status code is not present', () => { - const response = { foo: 'bar' }; - expect(() => handleGoCardlessError(response)).not.toThrow(); + it('throws a generic error when the status code is not recognised', () => { + const response = { response: { status: 0 } }; + expect(() => handleGoCardlessError(response)).toThrow( + GenericGoCardlessError, + ); }); }); diff --git a/upcoming-release-notes/439.md b/upcoming-release-notes/439.md new file mode 100644 index 0000000000..af0a7673fc --- /dev/null +++ b/upcoming-release-notes/439.md @@ -0,0 +1,6 @@ +--- +category: Bugfix +authors: [matt-fidd] +--- + +Fix GoCardless error handling