Skip to content

Commit

Permalink
Fix a race condition whereby the healthcheck is reporting good becaus…
Browse files Browse the repository at this point in the history
…e it hasn't run yet
  • Loading branch information
mattdean-digicatapult committed Jul 21, 2022
1 parent 4935b73 commit 601bb36
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 17 deletions.
5 changes: 3 additions & 2 deletions app/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ async function createHttpServer() {
const api = await createNodeApi()

const sw = new ServiceWatcher({
substrate: { healthCheck: () => nodeHealthCheck({ api, isReady: false }) },
substrate: { healthCheck: () => nodeHealthCheck(api._api) },
ipfs: { healthCheck: () => ipfsHealthCheack(ipfs) },
})
await sw.start()

await setupKeyWatcher({
api,
Expand Down Expand Up @@ -60,14 +61,14 @@ async function startServer() {
const server = app.listen(PORT, (err) => {
if (err) return reject(err)
logger.info(`Listening on port ${PORT} `)
sw.start()
resolve(server)
})

server.on('error', (err) => reject(err))
})

const closeHandler = (exitCode) => async () => {
sw.gen.return()
server.close(async () => {
await ipfs.stop()
process.exit(exitCode)
Expand Down
20 changes: 13 additions & 7 deletions app/utils/ServiceWatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,29 +44,35 @@ class ServiceWatcher {
.filter(Boolean)
}

// fire and forget, cancel using ServiceWatcher.gen.return()
// or ServiceWatcher.gen.throw(<instance of error>)
start() {
// starts the generator resolving after the first update
// use ServiceWatcher.gen.return() to stop
async start() {
if (this.services.length < 1) return null
this.gen = this.#generator()

const recursive = async (getAll = Promise.resolve([])) => {
const update = async (getAll = Promise.resolve([])) => {
try {
const services = await getAll
services.forEach(({ name, ...rest }) => this.update(name, rest))
await this.delay(this.#pollPeriod)
} catch (error) {
// if no service assume that this is server error e.g. TypeError, Parse...
const name = error.service || 'server'
this.update(name, { error, status: 'error' })
}
}

const recursive = async () => {
await this.delay(this.#pollPeriod)
const { value } = this.gen.next()
if (value) return recursive(value)
if (value) {
await update(value)
return recursive()
}
}

const { value } = this.gen.next()
recursive(value)
await update(value)
recursive()
}

// a generator function that returns poll fn for each service
Expand Down
4 changes: 2 additions & 2 deletions helm/dscp-ipfs/Chart.yaml
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
apiVersion: v2
name: dscp-ipfs
appVersion: '2.6.0'
appVersion: '2.6.1'
description: A Helm chart for dscp-ipfs
version: '2.6.0'
version: '2.6.1'
type: application
dependencies:
- name: dscp-node
Expand Down
2 changes: 1 addition & 1 deletion helm/dscp-ipfs/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ statefulSet:
image:
repository: digicatapult/dscp-ipfs
pullPolicy: IfNotPresent
tag: 'v2.6.0'
tag: 'v2.6.1'

storage:
storageClass: ""
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@digicatapult/dscp-ipfs",
"version": "2.6.0",
"version": "2.6.1",
"description": "Service for DSCP",
"main": "app/index.js",
"scripts": {
Expand Down
4 changes: 2 additions & 2 deletions test/unit/ServiceWatcher.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,11 +233,11 @@ describe('ServiceWatcher', function () {
beforeEach(async () => {
SW = new ServiceWatcher({ substrate: substrate.timeout })
spy(SW, 'update')
SW.start() // using await so it hits timeout
await SW.start() // using await so it hits timeout
await SW.delay(3000)
})

it('creates an instace of timeout error with error message', () => {
it('creates an instance of timeout error with error message', () => {
const { error } = SW.report.substrate

expect(error).to.be.a.instanceOf(TimeoutError)
Expand Down

0 comments on commit 601bb36

Please sign in to comment.