Repository navigation
Conversation
`src/config.js` had `module.exports.reload = reload;` where the
`module.exports = { ... }` opener belongs, leaving every property below it
orphaned — the module was a syntax error, so `require('./src/config')` threw and
anything that reached it through `src/index.js` failed to load. Jest collected
426 of 713 tests and 34 suites failed with "Jest encountered an unexpected token"
pointing at `src/index.js:6`.
The properties are assigned onto the existing exports object instead of
replacing it: `reload()` mutates that object in place (`Object.assign(config, …)`)
and `config` is bound to `module.exports` earlier in the file, so replacing it
would have left the hot-reload path updating an object nobody holds. `reload`
stays exported as `module.exports.reload`.
Verified: `node -e "require('./src/config')"` loads and `config.reload` is a
function; `npx jest` goes from `34 failed / 22 passed` suites (426 tests
collected) to `9 failed / 48 passed` (713 collected) — the remaining failures are
in files this commit does not touch.
…martDropLabs#376) `/health` reported `{configured: true, checked: false, status: 'unused'}` after checking only that a connection string was configured, so the field read as "the database is fine" while saying nothing about whether it could be reached. `checkDatabase()` now runs a real probe — a short-lived connection and `SELECT 1` — and reports what happened: - `unavailable` — no connection string configured, nothing probed. - `ok` — connected and answered, with `latency_ms`. - `error` — failed or exceeded the timeout, with the reason in `error`. The probe is bounded (2s default, `DEFAULT_TIMEOUT_MS`) so a hung database cannot hang `/health` itself; the endpoint still answers, and answering slowly is the failure this check exists to expose. `checkDatabase` accepts an injected `ping` so the states are testable without a database, and the endpoint already treats `status === 'error'` as unhealthy, so a database that cannot be reached now shows up there instead of being silently reported as unused — if you'd rather have that land as `degraded`, it is a one-line change in the status computation. Before this change the app did not load at all: `src/config.js` had `module.exports.reload = reload;` sitting where the `module.exports = { ... }` opener belongs, so every property below it was orphaned and the module was a syntax error. Jest collected 426 of 713 tests (34 failing suites, most of them "Jest encountered an unexpected token" pointing at `src/index.js:6`). The fix assigns onto the existing exports object rather than replacing it, because `reload()` mutates it in place and `config` was bound to it earlier in the file. With that one fix 287 more tests run. Test evidence (`npx jest`, both arms on this machine): - before: `Tests: 56 failed, 370 passed, 426 total` — `Test Suites: 34 failed, 22 passed` - after: `Tests: 56 failed, 657 passed, 713 total` — `Test Suites: 9 failed, 48 passed` The 51 tests that fail now and did not fail before are in suites that could not be loaded before the config fix (webhooks routes/dispatcher, auth, airdrops, priceOracle, indexer backoff) — same assertions, same untouched files. The health suite's failures in the before arm were an artifact of the parse error and pass now. Nothing in the areas this PR touches fails. Added: - `test/dbHealth.test.js` — 6 tests: ok with latency, error with the reason, timeout error that still resolves, unavailable with the probe never called, and two over a mocked `pg` proving the default probe connects to the configured URL, runs `SELECT 1` and closes the connection (including when the query throws). - `test/health.test.js` — the endpoint tests now inject the probe result instead of pointing at a database; the old `configured-but-unused` expectation is replaced by the probe result, plus a case asserting a failed probe surfaces as `unhealthy` with the reason. Also mocked `jobs/airdropExpiry` in the "status is ok" test: without it the airdrop job reports `healthy: false` (it never ran in the test process) and the endpoint answers `degraded`, which is why that test was red before this PR. README: the `/health` field table and the sample response no longer describe the `unused` state.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #376
/healthsaid{configured: true, checked: false, status: "unused"}after checking only that a connection string was configured, so the field read as "the database is fine" without ever asking the database. It now runs a real probe — short-lived connection,SELECT 1:unavailablewhen nothing is configured (no probe attempted)okwithlatency_mserrorwith the reasonThe probe is bounded (2s,
DEFAULT_TIMEOUT_MS) so a hung database cannot hang/health; answering slowly is what this check is for.checkDatabase()takes an injectedping, so the states are testable without a database. The endpoint already treatsstatus === "error"as unhealthy, so an unreachable database now surfaces there instead of hiding behindunused— say the word if you'd rather have it land asdegradedand I'll move it, it's one line in the status computation.One thing I had to fix first: the app did not load on
main.src/config.jshadmodule.exports.reload = reload;where themodule.exports = { ... }opener belongs, so every property under it was orphaned and the module was a syntax error. Jest collected 426 of 713 tests and 34 suites died withJest encountered an unexpected tokenpointing atsrc/index.js:6. I assigned onto the existing exports object instead of replacing it, becausereload()mutates it in place andconfigis bound tomodule.exportsearlier in the file — replacing it would leave the hot-reload path updating an object nobody holds.The 51 tests that fail now but not before are in suites that could not be loaded before the config fix (webhooks dispatcher/routes, auth, airdrops, priceOracle, indexer backoff) — same assertions, same untouched files, not caused by this change. The health suite's failures in the "before" arm were the parse error; they pass now.
What this PR adds:
test/dbHealth.test.js(6 tests): ok with latency, error with the reason, a timeout that still resolves,unavailablewith the probe never called, and two over a mockedpgproving the default probe connects to the configured URL, runsSELECT 1and closes the connection — including when the query throws.test/health.test.js: endpoint tests inject the probe result instead of pointing at a database; the oldconfigured-but-unusedexpectation is replaced by the probe result, plus a case asserting a failed probe surfaces asunhealthywith the reason. I also mockedjobs/airdropExpiryin the "status is ok" test — without it that job reportshealthy: false(it never ran in the test process) and the endpoint answersdegraded, which is why that test was already red./healthfield table and sample response no longer describeunused.Commands run:
How this was verified: this repository runs no CI on pull requests, so the checks list is empty rather than green — that is the repository's setup, not a missing step on my side. Everything below was run locally on this branch when the PR was opened:
Reproducible from the branch as it stands, if you want to confirm before merging.