Skip to content

Commit b592866

Browse files
committed
Modular server: improves logging
1 parent 63dc573 commit b592866

File tree

5 files changed

+29
-19
lines changed

5 files changed

+29
-19
lines changed

bin/www

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
#!/usr/bin/env node
22

3-
const http = require('http');
4-
const fs = require("fs");
5-
const path = require("path");
3+
const http = require('node:http');
4+
const fs = require("node:fs");
5+
const path = require("node:path");
66
const {ArgumentParser} = require("argparse");
7-
const { stat } = require('node:fs/promises');
87
const appFactory = require('../lib/appFactory');
98
const {configureLogger, getLogger} = require("../lib/logger");
109
const S3StoreRouter = require("../lib/routes/S3_store_router");
11-
const process = require("process");
12-
const https = require("https");
10+
const process = require("node:process");
11+
const https = require("node:https");
1312
const errToMessages = require("../lib/util/errToMessages");
1413

1514
const SSL_CIPHERS = 'ECDHE-RSA-AES256-SHA384:AES256-SHA256:RC4-SHA:RC4:HIGH:!MD5:!aNULL:!EDH:!AESGCM';
@@ -82,7 +81,10 @@ async function start(network) {
8281

8382
if (conf.trust_proxy) {
8483
app.set('trust proxy', conf.trust_proxy);
84+
} else if (process.env.NODE_ENV === 'production') {
85+
app.set('trust proxy', 'loopback'); // required for secure cookies + session
8586
}
87+
getLogger().info(`trust proxy: ${app.get('trust proxy')}`);
8688

8789
app.locals.title = "Modular Armadietto";
8890
// Before rendering, `locals.host` should be set to `getHost(req)`
@@ -222,7 +224,10 @@ function clientError (err, socket) {
222224
message = 'Bad Request';
223225
}
224226
const logNotes = errToMessages(err, new Set([message]));
225-
getLogger().warning(`- - - - - ${status} - “${Array.from(logNotes).join(' ')}”`);
227+
if (Number.isFinite(err.bytesParsed)) {
228+
logNotes.add(`${err.bytesParsed} bytes parsed`);
229+
}
230+
getLogger().warning(`${socket.address?.()?.address || '-'} - - - - ${status} - «${Array.from(logNotes).join(' ')}»`);
226231

227232
if (err.code !== 'ECONNRESET' && socket.writable) {
228233
socket.end(`HTTP/1.1 ${status} ${message}\r\n\r\n`);

lib/appFactory.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ module.exports = async function ({ hostIdentity, jwtSecret, accountMgr, storeRou
4242

4343
if (process.env.NODE_ENV === 'production') {
4444
app.use(rateLimiterMiddleware);
45+
getLogger().info('rateLimiterMiddleware enabled');
4546
}
4647

4748
// web browsers ask for this way too often, so doesn't log this
@@ -140,10 +141,6 @@ module.exports = async function ({ hostIdentity, jwtSecret, accountMgr, storeRou
140141
saveUninitialized: false,
141142
name: 'id'
142143
});
143-
if (process.env.NODE_ENV === 'production' && !app.get('trust proxy')) {
144-
app.set('trust proxy', 'loopback'); // required for secure cookies
145-
}
146-
getLogger().info(`trust proxy: ${app.get('trust proxy')}`);
147144

148145
app.use([`${basePath}/`, `${basePath}/oauth`, `${basePath}/account`, `${basePath}/admin`], memorySession);
149146

lib/middleware/rateLimiterMiddleware.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
// the capacity of all the instances. (Rate-limits are enforced
99
// per-instance.)
1010

11+
const { debuglog } = require('node:util');
12+
const { loggingMiddleware } = require('../logger');
1113
const { RateLimiterMemory, BurstyRateLimiter } = require('rate-limiter-flexible');
1214

1315
const POINTS_AUTH_REQUEST = 1;
@@ -20,6 +22,8 @@ const SUSTAINED_POINTS_PER_SEC = 8 * POINTS_UNAUTH_REQUEST;
2022

2123
// remotestorage.js appears to keep requesting until 10 failures
2224

25+
const debugEnabled = debuglog('ratelimiter').enabled;
26+
2327
const rateLimiterSustained = new RateLimiterMemory({
2428
points: SUSTAINED_POINTS_PER_SEC,
2529
duration: 1
@@ -76,7 +80,10 @@ const rateLimiterMiddleware = async (req, res, next) => {
7680
await rateLimiter.consume(req.ip, POINTS_UNAUTH_REQUEST);
7781
next();
7882
} catch (err) {
79-
// await new Promise(resolve => loggingMiddleware(req, res, resolve));
83+
if (debugEnabled) {
84+
await new Promise(resolve => loggingMiddleware(req, res, resolve));
85+
res.logNotes.add(`consumedPoints=${err.consumedPoints} msBeforeNext=${err.msBeforeNext}`);
86+
}
8087
res.set({ 'Retry-After': Math.ceil(err.msBeforeNext / 1000) });
8188
res.status(429).end();
8289
}

lib/routes/S3_store_router.js

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ module.exports = function ({ endPoint = 'play.min.io', accessKey = 'Q3AM3UQ867SP
176176
if (req.get('If-None-Match') && stripQuotes(req.get('If-None-Match')) === digest) {
177177
return res.status(304).end();
178178
} else if (req.get('If-Match') && stripQuotes(req.get('If-Match')) !== digest) {
179+
res.logNotes.add(`If-Match:${req.get('If-Match')} actually ${digest}`);
179180
return res.status(412).end();
180181
}
181182
res.set('ETag', normalizeETag(`"${digest}"`));
@@ -523,11 +524,11 @@ module.exports = function ({ endPoint = 'play.min.io', accessKey = 'Q3AM3UQ867SP
523524
res.logNotes.add(currentETag);
524525
res.status(412).end(); return;
525526
} else if (req.get('If-None-Match') && req.get('If-None-Match') === currentETag) {
526-
res.logNotes.add('If-None-Match:' + req.get('If-None-Match'));
527+
res.logNotes.add(`version ${currentETag} is already here`);
528+
res.logLevel = 'info';
527529
res.status(412).end(); return;
528530
} else if (req.get('If-Match') && req.get('If-Match') !== currentETag) {
529-
res.logNotes.add('If-Match:' + req.get('If-Match'));
530-
res.logNotes.add(currentETag);
531+
res.logNotes.add(`If-Match:${req.get('If-Match')} actually ${currentETag}`);
531532
res.status(412).end(); return;
532533
} // else unconditional
533534

@@ -638,12 +639,10 @@ module.exports = function ({ endPoint = 'play.min.io', accessKey = 'Q3AM3UQ867SP
638639
// And we have the info to check this.
639640
currentETag = normalizeETag(keys?.[0]?.ETag);
640641
if (req.get('If-Match') && req.get('If-Match') !== currentETag) {
641-
res.logNotes.add('If-Match:' + req.get('If-Match'));
642-
res.logNotes.add(currentETag);
642+
res.logNotes.add(`If-Match:${req.get('If-Match')} actually ${currentETag}`);
643643
return res.status(412).end();
644644
} else if (req.get('If-None-Match') && req.get('If-None-Match') === currentETag) {
645645
res.logNotes.add('If-None-Match:' + req.get('If-None-Match'));
646-
res.logNotes.add(currentETag);
647646
return res.status(412).end();
648647
}
649648

lib/util/errToMessages.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
const { getSystemErrorName } = require('node:util');
2+
13
/**
24
* Extracts & de-dupes payloads of error tree, for terse logging
35
* @param {Error} err
@@ -19,7 +21,7 @@ module.exports = function errToMessages (err, messages) {
1921
if (err.code && !Array.from(messages).some(msg => typeof msg === 'string' && msg?.includes(err.code))) {
2022
messages.add(err.code);
2123
}
22-
const errno = err.errno ? String(err.errno) : '';
24+
const errno = err.errno ? getSystemErrorName(err.errno) : '';
2325
if (errno && !Array.from(messages).some(msg => typeof msg === 'string' && msg?.includes(errno))) {
2426
messages.add(errno);
2527
}

0 commit comments

Comments
 (0)