From 9eac6deedd275908e8233445d732d828ad59880c Mon Sep 17 00:00:00 2001 From: Arran Standish Date: Wed, 13 Sep 2023 11:12:06 +0200 Subject: [PATCH 1/5] Set processing transactions to failed on startup --- src/middleware/messageStore.js | 8 +------ src/model/transactions.js | 38 +++++++++++++++++++++++++++------- src/server.js | 3 +++ test/unit/transactionsTest.js | 33 ++++++++++++++++++++++++++++- 4 files changed, 67 insertions(+), 15 deletions(-) diff --git a/src/middleware/messageStore.js b/src/middleware/messageStore.js index 60c91869..827d3a35 100644 --- a/src/middleware/messageStore.js +++ b/src/middleware/messageStore.js @@ -8,13 +8,7 @@ import * as metrics from '../metrics' import * as transactions from '../model/transactions' import * as utils from '../utils' -export const transactionStatus = { - PROCESSING: 'Processing', - SUCCESSFUL: 'Successful', - COMPLETED: 'Completed', - COMPLETED_W_ERR: 'Completed with error(s)', - FAILED: 'Failed' -} +const { transactionStatus } = transactions function copyMapWithEscapedReservedCharacters(map) { const escapedMap = {} diff --git a/src/model/transactions.js b/src/model/transactions.js index f1e2760b..533c0a86 100644 --- a/src/model/transactions.js +++ b/src/model/transactions.js @@ -4,6 +4,14 @@ import {Schema} from 'mongoose' import {connectionAPI, connectionDefault} from '../config' +export const transactionStatus = { + PROCESSING: 'Processing', + SUCCESSFUL: 'Successful', + COMPLETED: 'Completed', + COMPLETED_W_ERR: 'Completed with error(s)', + FAILED: 'Failed' +} + // Request Schema definition const RequestDef = { host: String, @@ -94,13 +102,7 @@ const TransactionSchema = new Schema({ status: { type: String, required: true, - enum: [ - 'Processing', - 'Failed', - 'Completed', - 'Successful', - 'Completed with error(s)' - ] + enum: Object.values(transactionStatus) } }) @@ -118,3 +120,25 @@ export const TransactionModel = connectionDefault.model( 'Transaction', TransactionSchema ) + +/** + * Resolve a transaction stuck in the processing state + * + * If OpenHIM crashes with an inflight transaction, that transaction's status will stay in processing + * So we run this function at start up and set all those transactions to failed + * + */ +export const resolveStuckProcessingState = async () => { + TransactionModelAPI.find({ status: transactionStatus.PROCESSING }) + .cursor() + .on('data', async (transaction) => { + try { + TransactionModelAPI.findByIdAndUpdate(transaction.id, { + status: transactionStatus.FAILED, + error: { message: 'Application failed while still waiting on a response' }, + }).exec() + } catch (err) { + console.error(`Error updating transaction stuck in processing: ${err}`) + } + }) +} diff --git a/src/server.js b/src/server.js index 11267547..334832ab 100644 --- a/src/server.js +++ b/src/server.js @@ -37,6 +37,7 @@ import * as upgradeDB from './upgradeDB' import {KeystoreModel} from './model/keystore' import {UserModel, createUser, updateTokenUser} from './model/users' import {appRoot, config, connectionAgenda} from './config' +import { resolveStuckProcessingState } from './model/transactions' mongoose.Promise = Promise @@ -886,6 +887,8 @@ if (cluster.isMaster && !module.parent) { return Promise.all(promises) .then(() => { + resolveStuckProcessingState() + let audit = atna.construct.appActivityAudit( true, himSourceID, diff --git a/test/unit/transactionsTest.js b/test/unit/transactionsTest.js index da332299..567bb4f4 100644 --- a/test/unit/transactionsTest.js +++ b/test/unit/transactionsTest.js @@ -3,7 +3,7 @@ /* eslint-env mocha */ import * as transactions from '../../src/api/transactions' -import {TaskModel, TransactionModel} from '../../src/model' +import {TaskModel, TransactionModel, resolveStuckProcessingState} from '../../src/model' import {ObjectId} from 'mongodb' describe('calculateTransactionBodiesByteLength()', () => { @@ -89,3 +89,34 @@ describe('*createRerunTasks', () => { tasks.length.should.be.exactly(2) }) }) + +describe('TransactionModel tests', () => { + const transaction = Object.freeze({ + status: 'Processing', + request: { + timestamp: new Date().toISOString() + }, + updatedBy: { + id: new ObjectId(), + name: 'Test' + } + }) + + beforeEach(async () => { + await TransactionModel.deleteMany({}) + }) + + afterEach(async () => { + await TransactionModel.deleteMany({}) + }) + + describe('.resolveStuckProcessingState()', async () => { + await TransactionModel(transaction).save() + + resolveStuckProcessingState() + await new Promise((resolve) => setTimeout(() => { resolve() }, 1000)) + + const transactions = await TransactionModel.find({ status: 'Processing' }); + transactions.length.should.be.exactly(0); + }) +}) \ No newline at end of file From 0655df929fbe33075ff97abee2ddefea97d7f00e Mon Sep 17 00:00:00 2001 From: Arran Standish Date: Wed, 13 Sep 2023 11:13:35 +0200 Subject: [PATCH 2/5] Update version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1cf9bd5f..fc1041b5 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "openhim-core", "description": "The OpenHIM core application that provides logging and routing of http requests", - "version": "8.1.1", + "version": "8.1.2", "main": "./lib/server.js", "bin": { "openhim-core": "./bin/openhim-core.js" From 70921616b36043cea52565ece9fe78cbd9bd2c3d Mon Sep 17 00:00:00 2001 From: Arran Standish Date: Wed, 13 Sep 2023 11:29:35 +0200 Subject: [PATCH 3/5] Improve error message that gets set --- src/model/transactions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/model/transactions.js b/src/model/transactions.js index 533c0a86..6f2459f9 100644 --- a/src/model/transactions.js +++ b/src/model/transactions.js @@ -135,7 +135,7 @@ export const resolveStuckProcessingState = async () => { try { TransactionModelAPI.findByIdAndUpdate(transaction.id, { status: transactionStatus.FAILED, - error: { message: 'Application failed while still waiting on a response' }, + error: { message: 'OpenHIM crashed while still waiting for a response' }, }).exec() } catch (err) { console.error(`Error updating transaction stuck in processing: ${err}`) From 4bfd93e5aeb256f2fd9b8a37de636b8cc1652ed9 Mon Sep 17 00:00:00 2001 From: arran-standish <125864621+arran-standish@users.noreply.github.com> Date: Wed, 13 Sep 2023 12:00:18 +0200 Subject: [PATCH 4/5] Add end of file line --- test/unit/transactionsTest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/transactionsTest.js b/test/unit/transactionsTest.js index 567bb4f4..0aad8873 100644 --- a/test/unit/transactionsTest.js +++ b/test/unit/transactionsTest.js @@ -119,4 +119,4 @@ describe('TransactionModel tests', () => { const transactions = await TransactionModel.find({ status: 'Processing' }); transactions.length.should.be.exactly(0); }) -}) \ No newline at end of file +}) From 62112df4c54bace0151af4935bfb26e78ec4877c Mon Sep 17 00:00:00 2001 From: Arran Standish Date: Thu, 14 Sep 2023 08:29:56 +0200 Subject: [PATCH 5/5] Only update transactions without a response or error (so only in-flight) --- src/model/transactions.js | 9 ++-- test/unit/transactionsTest.js | 97 +++++++++++++++++++++++++---------- 2 files changed, 75 insertions(+), 31 deletions(-) diff --git a/src/model/transactions.js b/src/model/transactions.js index 6f2459f9..9b21fc58 100644 --- a/src/model/transactions.js +++ b/src/model/transactions.js @@ -133,10 +133,11 @@ export const resolveStuckProcessingState = async () => { .cursor() .on('data', async (transaction) => { try { - TransactionModelAPI.findByIdAndUpdate(transaction.id, { - status: transactionStatus.FAILED, - error: { message: 'OpenHIM crashed while still waiting for a response' }, - }).exec() + if (transaction.$isEmpty('response') && transaction.$isEmpty('error')) + TransactionModelAPI.findByIdAndUpdate(transaction.id, { + status: transactionStatus.FAILED, + error: { message: 'OpenHIM crashed while still waiting for a response' }, + }).exec() } catch (err) { console.error(`Error updating transaction stuck in processing: ${err}`) } diff --git a/test/unit/transactionsTest.js b/test/unit/transactionsTest.js index 567bb4f4..94ff585f 100644 --- a/test/unit/transactionsTest.js +++ b/test/unit/transactionsTest.js @@ -91,32 +91,75 @@ describe('*createRerunTasks', () => { }) describe('TransactionModel tests', () => { - const transaction = Object.freeze({ - status: 'Processing', - request: { - timestamp: new Date().toISOString() - }, - updatedBy: { - id: new ObjectId(), - name: 'Test' - } - }) - - beforeEach(async () => { - await TransactionModel.deleteMany({}) - }) - - afterEach(async () => { - await TransactionModel.deleteMany({}) - }) - - describe('.resolveStuckProcessingState()', async () => { - await TransactionModel(transaction).save() - - resolveStuckProcessingState() - await new Promise((resolve) => setTimeout(() => { resolve() }, 1000)) - - const transactions = await TransactionModel.find({ status: 'Processing' }); - transactions.length.should.be.exactly(0); + describe('.resolveStuckProcessingState()', () => { + const midFlightTransaction = Object.freeze({ + status: 'Processing', + request: { + timestamp: new Date().toISOString() + }, + updatedBy: { + id: new ObjectId(), + name: 'Test' + } + }) + + const validProcessingTransaction = Object.freeze({ + status: 'Processing', + request: { + timestamp: new Date().toISOString() + }, + response: { + status: 200, + timestamp: new Date().toISOString() + }, + updatedBy: { + id: new ObjectId(), + name: 'Test' + } + }) + + const errorProcessingTransaction = Object.freeze({ + status: 'Processing', + request: { + timestamp: new Date().toISOString() + }, + error: { + message: 'something bad happened', + stack: 'stack trace' + }, + updatedBy: { + id: new ObjectId(), + name: 'Test' + } + }) + + beforeEach(async () => { + await TransactionModel.deleteMany({}) + }) + + afterEach(async () => { + await TransactionModel.deleteMany({}) + }) + + it('should update a processing transaction to failed if no response or error set', async () => { + await TransactionModel(midFlightTransaction).save() + + resolveStuckProcessingState() + await new Promise((resolve) => setTimeout(() => { resolve() }, 500)) + + const transactions = await TransactionModel.find({ status: 'Processing' }); + transactions.length.should.be.exactly(0); + }) + + it('should not update a transaction processing state if response or error set', async () => { + await TransactionModel(validProcessingTransaction).save() + await TransactionModel(errorProcessingTransaction).save() + + resolveStuckProcessingState() + await new Promise((resolve) => setTimeout(() => { resolve() }, 500)) + + const transactions = await TransactionModel.find({ status: 'Processing' }); + transactions.length.should.be.exactly(2); + }) }) }) \ No newline at end of file