From fb3fc62b4be99141ecfe8c1055d836ee3dc0d1c0 Mon Sep 17 00:00:00 2001 From: Sjoerd de Vries Date: Sat, 5 Sep 2026 15:52:57 +0200 Subject: [PATCH] feat: log failures, validate env at startup, load .env (findings 3, 4, 7) Finding 3: fetch_error, not_found, mail_error and de onverwachte-fout branch loggen nu naam/url plus reden naar stderr; runCheck logt na elke run een regel met de tellingen per status, zodat een cron-run zichtbaar is in de container-logs. De recordCheckSuccess/recordCheckFailure-condities en alle return-waarden zijn ongewijzigd. Finding 4: server.js controleert bij het opstarten dat BASIC_AUTH_USER, BASIC_AUTH_PASS, SMTP_FROM en NOTIFY_EMAIL gezet zijn en stopt anders met exit code 1 in plaats van later opake 500's te geven. Finding 7: dotenv toegevoegd en als eerste regel in server.js geladen, zodat het gedocumenteerde `cp .env.example .env && npm start` echt werkt. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_013gwc3aCiyQxmCWQke8RvEz --- README.md | 4 ++++ package-lock.json | 13 +++++++++++++ package.json | 1 + server.js | 26 +++++++++++++++++++++++++- src/checkProducts.js | 6 +++++- test/checkProducts.test.js | 26 ++++++++++++++++++++++---- 6 files changed, 70 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index b2a2c3d..42e909b 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,10 @@ npm test npm start ``` +De app leest `.env` in via `dotenv`. `BASIC_AUTH_USER`, `BASIC_AUTH_PASS`, +`SMTP_FROM` en `NOTIFY_EMAIL` zijn verplicht; ontbreken ze, dan stopt de +app bij het opstarten met een melding. + De UI staat achter basic auth (`BASIC_AUTH_USER` / `BASIC_AUTH_PASS`) op `http://localhost:3000`. diff --git a/package-lock.json b/package-lock.json index 85242f9..0881b36 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,6 +10,7 @@ "dependencies": { "better-sqlite3": "^11.3.0", "cheerio": "^1.0.0", + "dotenv": "^17.4.2", "express": "^4.19.2", "express-basic-auth": "^1.2.1", "node-cron": "^3.0.3", @@ -527,6 +528,18 @@ "url": "https://github.com/fb55/domutils?sponsor=1" } }, + "node_modules/dotenv": { + "version": "17.4.2", + "resolved": "https://registry.npmjs.org/dotenv/-/dotenv-17.4.2.tgz", + "integrity": "sha512-nI4U3TottKAcAD9LLud4Cb7b2QztQMUEfHbvhTH09bqXTxnSie8WnjPALV/WMCrJZ6UV/qHJ6L03OqO3LcdYZw==", + "license": "BSD-2-Clause", + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://dotenvx.com" + } + }, "node_modules/dunder-proto": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/dunder-proto/-/dunder-proto-1.0.1.tgz", diff --git a/package.json b/package.json index 8dd53d6..98d94e1 100644 --- a/package.json +++ b/package.json @@ -11,6 +11,7 @@ "dependencies": { "better-sqlite3": "^11.3.0", "cheerio": "^1.0.0", + "dotenv": "^17.4.2", "express": "^4.19.2", "express-basic-auth": "^1.2.1", "node-cron": "^3.0.3", diff --git a/server.js b/server.js index f45c20a..724f511 100644 --- a/server.js +++ b/server.js @@ -1,3 +1,7 @@ +// Leest een .env-bestand in voor lokale ontwikkeling; in productie komen de +// env vars van Coolify. quiet houdt de opstartlogging schoon. +require('dotenv').config({ quiet: true }); + const path = require('node:path'); const { openDb, listProducts, addProduct, deleteProduct, @@ -9,12 +13,29 @@ const { checkAllProducts } = require('./src/checkProducts'); const { start: startScheduler } = require('./src/scheduler'); const { createApp } = require('./src/app'); +const REQUIRED_ENV = ['BASIC_AUTH_USER', 'BASIC_AUTH_PASS', 'SMTP_FROM', 'NOTIFY_EMAIL']; +const missing = REQUIRED_ENV.filter((name) => !process.env[name]); +if (missing.length > 0) { + console.error(`Ontbrekende verplichte environment variables: ${missing.join(', ')}`); + console.error('Zet ze (bijvoorbeeld in .env, zie .env.example) en start opnieuw.'); + process.exit(1); +} + const PORT = process.env.PORT || 3000; const DB_PATH = process.env.DB_PATH || path.join(__dirname, 'data', 'korting.db'); const db = openDb(DB_PATH); const transport = createTransport(process.env); +function summarize(results) { + const counts = {}; + for (const result of results) { + counts[result.status] = (counts[result.status] || 0) + 1; + } + const parts = Object.entries(counts).map(([status, count]) => `${status}=${count}`); + return parts.length > 0 ? parts.join(' ') : 'geen producten'; +} + function runCheck() { const products = listProducts(db); const deps = { @@ -28,7 +49,10 @@ function runCheck() { recordCheckSuccess: (id, data) => recordCheckSuccess(db, id, data), now: () => new Date().toISOString(), }; - return checkAllProducts(products, deps); + return checkAllProducts(products, deps).then((results) => { + console.log(`[check] ${products.length} product(en) gecontroleerd: ${summarize(results)}`); + return results; + }); } startScheduler(runCheck); diff --git a/src/checkProducts.js b/src/checkProducts.js index ac9a988..c0dff43 100644 --- a/src/checkProducts.js +++ b/src/checkProducts.js @@ -3,10 +3,12 @@ async function checkProduct(product, deps) { const result = await fetchAndExtractPrice(product.url); if (result.error) { + console.error(`[check] fetch_error "${product.name}" (${product.url}): ${result.error.message}`); recordCheckFailure(product.id, 'fetch_error'); return { status: 'fetch_error' }; } if (result.price == null) { + console.error(`[check] not_found "${product.name}" (${product.url}): geen prijs gevonden op de pagina`); recordCheckFailure(product.id, 'not_found'); return { status: 'not_found' }; } @@ -22,7 +24,8 @@ async function checkProduct(product, deps) { oldPrice: product.last_price, newPrice: result.price, }); - } catch { + } catch (err) { + console.error(`[check] mail_error "${product.name}" (${product.url}): ${err && err.message}`); recordCheckFailure(product.id, 'mail_error'); return { status: 'mail_error' }; } @@ -39,6 +42,7 @@ async function checkAllProducts(products, deps) { const result = await checkProduct(product, deps); results.push({ productId: product.id, ...result }); } catch (err) { + console.error(`[check] error "${product.name}" (${product.url}): ${err && err.message}`); results.push({ productId: product.id, status: 'error', error: err }); } } diff --git a/test/checkProducts.test.js b/test/checkProducts.test.js index 1e7bd2b..03633eb 100644 --- a/test/checkProducts.test.js +++ b/test/checkProducts.test.js @@ -17,6 +17,14 @@ function makeDeps(overrides = {}) { return { deps, failures, successes, mailCalls }; } +// Failure paths log to stderr; capture it so the test output stays clean +// while still proving the logging happened. +function captureErrorLog(t) { + const logged = []; + t.mock.method(console, 'error', (...args) => { logged.push(args.join(' ')); }); + return logged; +} + test('checkProduct stores baseline price without sending mail', async () => { const { deps, successes, mailCalls } = makeDeps(); const product = { id: 1, name: 'X', url: 'https://x', last_price: null }; @@ -48,7 +56,8 @@ test('checkProduct does not send mail or update when price unchanged', async () assert.equal(successes.length, 1); }); -test('checkProduct records fetch_error and does not touch price on network failure', async () => { +test('checkProduct records fetch_error and does not touch price on network failure', async (t) => { + const logged = captureErrorLog(t); const { deps, failures, successes } = makeDeps({ fetchAndExtractPrice: async () => ({ price: null, method: null, error: new Error('down') }), }); @@ -57,9 +66,12 @@ test('checkProduct records fetch_error and does not touch price on network failu assert.equal(result.status, 'fetch_error'); assert.deepEqual(failures, [{ id: 4, status: 'fetch_error' }]); assert.equal(successes.length, 0); + assert.equal(logged.length, 1); + assert.match(logged[0], /fetch_error.*https:\/\/x.*down/); }); -test('checkProduct records not_found when no price could be extracted', async () => { +test('checkProduct records not_found when no price could be extracted', async (t) => { + const logged = captureErrorLog(t); const { deps, failures, successes } = makeDeps({ fetchAndExtractPrice: async () => ({ price: null, method: null, error: null }), }); @@ -68,9 +80,12 @@ test('checkProduct records not_found when no price could be extracted', async () assert.equal(result.status, 'not_found'); assert.deepEqual(failures, [{ id: 5, status: 'not_found' }]); assert.equal(successes.length, 0); + assert.equal(logged.length, 1); + assert.match(logged[0], /not_found.*https:\/\/x/); }); -test('checkProduct records mail_error and keeps old price when sending mail fails', async () => { +test('checkProduct records mail_error and keeps old price when sending mail fails', async (t) => { + const logged = captureErrorLog(t); const { deps, failures, successes } = makeDeps({ sendMail: async () => { throw new Error('smtp down'); }, }); @@ -79,9 +94,12 @@ test('checkProduct records mail_error and keeps old price when sending mail fail assert.equal(result.status, 'mail_error'); assert.deepEqual(failures, [{ id: 6, status: 'mail_error' }]); assert.equal(successes.length, 0); + assert.equal(logged.length, 1); + assert.match(logged[0], /mail_error.*https:\/\/x.*smtp down/); }); -test('checkAllProducts processes every product independently, isolating failures', async () => { +test('checkAllProducts processes every product independently, isolating failures', async (t) => { + captureErrorLog(t); const { deps } = makeDeps(); let calls = 0; const throwingDeps = {