Skip to content

Commit

Permalink
PP 10451 handle selfservice 504 ledger gateway timeout (#4150)
Browse files Browse the repository at this point in the history
* PP-10451 Handle gateway timeout error during transactions download.
This change is for the single service transaction search and not the all services transaction search functionality.

Converting test scenarios as Cypress Tests for 400, 500 and 504 responses and removing nock mocking logic.

Removing all nock stubbing logic from the relevant tests.

Download of transactions list by clicking the download link was de-scoped; we decided we weren’t going to change anything for CSV downloads, because the chance of encountering a timeout when getting the data for the CSV was unlikely enough not to bother treating specially.
  • Loading branch information
olatomgds authored Nov 1, 2023
1 parent 55d1a69 commit c463f0f
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 46 deletions.
Original file line number Diff line number Diff line change
@@ -1,28 +1,16 @@
'use strict'

const nock = require('nock')
const sinon = require('sinon')

const paths = require('../../paths')
const getQueryStringForParams = require('../../utils/get-query-string-for-params')
const formatAccountPathsFor = require('../../utils/format-account-paths-for')
const { validGatewayAccountResponse } = require('../../../test/fixtures/gateway-account.fixtures')
const transactionListController = require('./transaction-list.controller')

// Setup
const gatewayAccountId = '651342'
const ledgerSearchParameters = {}
const EXTERNAL_GATEWAY_ACCOUNT_ID = 'an-external-id'
const LEDGER_TRANSACTION_PATH = '/v1/transaction?account_id=' + gatewayAccountId
const requestId = 'unique-request-id'
const headers = { 'x-request-id': requestId }
const ledgerMock = nock(process.env.LEDGER_URL, { reqheaders: headers })

function ledgerMockResponds (code, data, searchParameters) {
const queryString = getQueryStringForParams(searchParameters)
return ledgerMock.get(LEDGER_TRANSACTION_PATH + '&' + queryString)
.reply(code, data)
}

describe('The /transactions endpoint', () => {
const account = validGatewayAccountResponse(
Expand All @@ -42,40 +30,17 @@ describe('The /transactions endpoint', () => {
const res = {}
let next

afterEach(() => {
nock.cleanAll()
})

beforeEach(() => {
next = sinon.spy()
})

describe('Error getting transactions', () => {
it('should show error message on a bad request while retrieving the list of transactions', async () => {
const errorMessage = 'There is a problem with the payments platform. Please contact the support team.'
ledgerMockResponds(400, { 'message': errorMessage }, ledgerSearchParameters)

await transactionListController(req, res, next)
const expectedError = sinon.match.instanceOf(Error)
.and(sinon.match.has('message', 'Unable to retrieve list of transactions or card types'))
sinon.assert.calledWith(next, expectedError)
})

it('should show a generic error message on a ledger service error while retrieving the list of transactions', async () => {
ledgerMockResponds(500, { 'message': 'some error from connector' }, ledgerSearchParameters)

await transactionListController(req, res, next)
const expectedError = sinon.match.instanceOf(Error)
.and(sinon.match.has('message', 'Unable to retrieve list of transactions or card types'))
sinon.assert.calledWith(next, expectedError)
})

it('should show internal error message if any error happens while retrieving the list of transactions', async () => {
// No ledgerMock defined on purpose to mock a network failure

// No mocking defined on purpose to mock a network failure,
// This integration test will cover server errors outside the 500 and 504 defined in the Cypress test
await transactionListController(req, res, next)
const expectedError = sinon.match.instanceOf(Error)
.and(sinon.match.has('message', 'Unable to retrieve list of transactions or card types'))
.and(sinon.match.has('message', 'Unable to retrieve list of transactions or card types.'))
sinon.assert.calledWith(next, expectedError)
})
})
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/transactions/transaction-list.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ module.exports = async function showTransactionList (req, res, next) {
transactionService.search([accountId], filters.result),
client.getAllCardTypes()
])
} catch (err) {
return next(new Error('Unable to retrieve list of transactions or card types'))
} catch (error) {
return next(error)
}

const transactionsDownloadLink = formatAccountPathsFor(router.paths.account.transactions.download, req.account.external_id)
Expand Down
11 changes: 10 additions & 1 deletion app/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ class DomainError extends Error {
}
}

class GatewayTimeoutError extends Error {
constructor (message) {
super(message)
this.name = this.constructor.name
Error.captureStackTrace(this, this.constructor)
}
}

/**
* Thrown when there is no authentication session for the user.
*/
Expand Down Expand Up @@ -96,5 +104,6 @@ module.exports = {
RegistrationSessionMissingError,
InvalidRegistationStateError,
InvalidConfigurationError,
ExpiredInviteError
ExpiredInviteError,
GatewayTimeoutError
}
8 changes: 7 additions & 1 deletion app/middleware/error-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ const {
InvalidRegistationStateError,
InvalidConfigurationError,
ExpiredInviteError,
RESTClientError
RESTClientError,
GatewayTimeoutError
} = require('../errors')
const paths = require('../paths')
const { renderErrorView, response } = require('../utils/response')
Expand Down Expand Up @@ -80,6 +81,11 @@ module.exports = function errorHandler (err, req, res, next) {
return renderErrorView(req, res, 'There is a problem with the payments platform. Please contact the support team', 400)
}

if (err instanceof GatewayTimeoutError) {
logger.info('Gateway Time out Error occurred on Transactions Search Page. Rendering error page')
return renderErrorView(req, res, err.message, 504)
}

if (err instanceof RESTClientError) {
logger.info(`Unhandled REST client error caught: ${err.message}`, {
service: err.service,
Expand Down
10 changes: 7 additions & 3 deletions app/services/transaction.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const getQueryStringForParams = require('../utils/get-query-string-for-params')
const userService = require('../services/user.service')
const transactionView = require('../utils/transaction-view')
const errorIdentifier = require('../models/error-identifier')
const { GatewayTimeoutError } = require('../errors')

const connector = new ConnectorClient(process.env.CONNECTOR_URL)

Expand All @@ -22,10 +23,13 @@ const connectorRefundFailureReasons = {

const searchLedger = async function searchLedger (gatewayAccountIds = [], filters) {
try {
const transactions = await Ledger.transactions(gatewayAccountIds, filters)
return transactions
return await Ledger.transactions(gatewayAccountIds, filters)
} catch (error) {
throw new Error('GET_FAILED')
if (error.errorCode === 504) {
throw new GatewayTimeoutError('Your request has timed out. Please apply more filters and try again.')
} else {
throw new Error('Unable to retrieve list of transactions or card types.')
}
}
}

Expand Down
122 changes: 122 additions & 0 deletions test/cypress/integration/transactions/transaction-search.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@ describe('Transactions List', () => {
cy.get('#download-transactions-link').should('have.attr', 'href', `/account/a-valid-external-id/transactions/download?dispute_states=needs_response&dispute_states=under_review`)
})
})

describe('csv download link', () => {
it('should not display csv download link when results >5k and no filter applied', function () {
cy.task('setupStubs', [
Expand Down Expand Up @@ -461,4 +462,125 @@ describe('Transactions List', () => {
cy.get('#download-transactions-link').should('exist')
})
})

describe('Should display relevant error page on search failure ', () => {
it('should show error message on a bad request while retrieving the list of transactions', () => {
cy.task('setupStubs', [
...sharedStubs(),
transactionsStubs.getLedgerTransactionsSuccess({ gatewayAccountId, transactions: unfilteredTransactions })
])
cy.visit(transactionsUrl, { failOnStatusCode: false })

cy.task('clearStubs')

cy.task('setupStubs', [
...sharedStubs(),
transactionsStubs.getLedgerTransactionsFailure(
{
account_id: gatewayAccountId,
limit_total: 'true',
limit_total_size: '5001',
from_date: '',
to_date: '',
page: '1',
display_size: '100'
},
400)
])

// Click the filter button
cy.get('#filter').click()

// Ensure that transaction list is not displayed
cy.get('#transactions-list tbody').should('not.exist')

// Ensure an error message header is displayed
cy.get('h1').contains('An error occurred')

// Ensure a generic error message is displayed
cy.get('#errorMsg').contains('There is a problem with the payments platform. Please contact the support team.')
})

it('should display the generic error page, if an internal server error occurs while retrieving the list of transactions', () => {
cy.task('setupStubs', [
...sharedStubs(),
transactionsStubs.getLedgerTransactionsSuccess({ gatewayAccountId, transactions: unfilteredTransactions })
])
cy.visit(transactionsUrl, { failOnStatusCode: false })

cy.task('clearStubs')

cy.task('setupStubs', [
...sharedStubs(),
transactionsStubs.getLedgerTransactionsFailure(
{
account_id: gatewayAccountId,
limit_total: 'true',
limit_total_size: '5001',
from_date: '',
to_date: '',
page: '1',
display_size: '100'
},
500)
])

// Click the filter button
cy.get('#filter').click()

// Ensure that transaction list is not displayed
cy.get('#transactions-list tbody').should('not.exist')

// Ensure an error message header is displayed
cy.get('h1').contains('An error occurred')

// Ensure a generic error message is displayed
cy.get('#errorMsg').contains('There is a problem with the payments platform. Please contact the support team.')
})

it('should display the gateway timeout error page, if a gateway timeout error occurs while retrieving the list of transactions', () => {
cy.task('setupStubs', [
...sharedStubs(),
transactionsStubs.getLedgerTransactionsSuccess({ gatewayAccountId, transactions: unfilteredTransactions })
])

cy.visit(transactionsUrl, { failOnStatusCode: false })

// Fill from and to date
cy.get('#fromDate').type('03/5/2018')
cy.get('#fromTime').type('01:00:00')
cy.get('#toDate').type('03/5/2023')
cy.get('#toTime').type('01:00:00')

// 1. Filtering
cy.task('clearStubs')

cy.task('setupStubs', [
...sharedStubs(),
transactionsStubs.getLedgerTransactionsFailure(
{
account_id: gatewayAccountId,
limit_total: 'true',
limit_total_size: '5001',
from_date: '2018-05-03T00:00:00.000Z',
to_date: '2023-05-03T00:00:01.000Z',
page: '1',
display_size: '100'
},
504)
])

// Click the filter button
cy.get('#filter').click()

// Ensure that transaction list is not displayed
cy.get('#transactions-list tbody').should('not.exist')

// Ensure an error message header is displayed
cy.get('h1').contains('An error occurred')

// Ensure a gateway timeout error message is displayed
cy.get('#errorMsg').contains('Your request has timed out. Please apply more filters and try again')
})
})
})
17 changes: 16 additions & 1 deletion test/cypress/stubs/transaction-stubs.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,27 @@ function getTransactionsSummarySuccess (opts) {
})
}

function getLedgerTransactionsFailure (opts, responseCode) {
const path = `/v1/transaction`
return stubBuilder('GET', path, responseCode, {
query: {
account_id: opts.account_id,
limit_total: opts.limit_total,
limit_total_size: opts.limit_total_size,
from_date: opts.from_date,
to_date: opts.to_date,
page: opts.page,
display_size: opts.display_size
} })
}

module.exports = {
getLedgerEventsSuccess,
getLedgerTransactionSuccess,
getLedgerTransactionsSuccess,
getLedgerDisputeTransactionsSuccess,
postRefundSuccess,
postRefundAmountNotAvailable,
getTransactionsSummarySuccess
getTransactionsSummarySuccess,
getLedgerTransactionsFailure
}

0 comments on commit c463f0f

Please sign in to comment.