Skip to content

Commit

Permalink
Merge pull request #1275 from FoalTS/deprecation-server-url
Browse files Browse the repository at this point in the history
Make logging more suitable for log monitoring softwares
  • Loading branch information
LoicPoullain authored Aug 3, 2024
2 parents 9489962 + 9917b55 commit 8299833
Show file tree
Hide file tree
Showing 13 changed files with 52 additions and 242 deletions.
6 changes: 6 additions & 0 deletions docs/blog/version-4.5-release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ Version 4.5 of [Foal](https://foalts.org/) is out!

<!--truncate-->

## Logging improvements

In previous versions, the util function `displayServerURL` and configuration errors printed logs on several lines, which was not appropriate for logging software.

From version 4.5 onwards, configuration errors are displayed on a single line and the `displayServerURL` function is marked as deprecated.

## CLI fixes

When running `npx foal connect react` to connect the React application to the Foal application in development, the following features did not work:
Expand Down
3 changes: 2 additions & 1 deletion docs/docs/common/websockets.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,14 @@ import * as http from 'http';

async function main() {
const serviceManager = new ServiceManager();
const logger = serviceManager.get(Logger);

const app = await createApp(AppController, { serviceManager });

const httpServer = http.createServer(app);
// Instanciate, init and connect websocket controllers.
await serviceManager.get(WebsocketController).attachHttpServer(httpServer);
httpServer.listen(port, () => displayServerURL(port));
httpServer.listen(port, () => logger.info(`Listening on port ${port}...`));
}

```
Expand Down
7 changes: 5 additions & 2 deletions docs/docs/security/rate-limiting.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ npm install express express-rate-limit
*src/index.ts*
```typescript
// 3p
import { Config, createApp, displayServerURL } from '@foal/core';
import { Config, createApp, Logger, ServiceManager } from '@foal/core';
import * as express from 'express';
import * as rateLimit from 'express-rate-limit';

Expand All @@ -41,11 +41,14 @@ async function main() {
res.status(this.statusCode || 429).send(this.message);
}
}));

const serviceManager = new ServiceManager();
const logger = serviceManager.get(Logger);

const app = await createApp(AppController, { expressInstance: expressApp });

const port = Config.get('port', 'number', 3001);
app.listen(port, () => displayServerURL(port));
app.listen(port, () => logger.info(`Listening on port ${port}...`));
}

main()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// 3p
import { Config, createApp, displayServerURL } from '@foal/core';
import { Config, createApp, Logger, ServiceManager } from '@foal/core';
import * as express from 'express';
import * as rateLimit from 'express-rate-limit';

Expand Down Expand Up @@ -28,9 +28,12 @@ it('[Docs] Cookbook > Limit Repeated Requests', () => {
}
}));

const serviceManager = new ServiceManager();
const logger = serviceManager.get(Logger);

const app = await createApp(AppController, { expressInstance: expressApp });

const port = Config.get('port', 'number', 3001);
app.listen(port, () => displayServerURL(port));
app.listen(port, () => logger.info(`Listening on port ${port}...`));
}
});
9 changes: 6 additions & 3 deletions packages/cli/src/generate/specs/app/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import 'source-map-support/register';

// 3p
import { Config, createApp, displayServerURL } from '@foal/core';
import { Config, createApp, Logger, ServiceManager } from '@foal/core';

// App
import { AppController } from './app/app.controller';
Expand All @@ -10,10 +10,13 @@ import { dataSource } from './db';
async function main() {
await dataSource.initialize();

const app = await createApp(AppController);
const serviceManager = new ServiceManager();
const logger = serviceManager.get(Logger);

const app = await createApp(AppController, { serviceManager });

const port = Config.get('port', 'number', 3001);
app.listen(port, () => displayServerURL(port));
app.listen(port, () => logger.info(`Listening on port ${port}...`));
}

main()
Expand Down
9 changes: 6 additions & 3 deletions packages/cli/src/generate/templates/app/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import 'source-map-support/register';

// 3p
import { Config, createApp, displayServerURL } from '@foal/core';
import { Config, createApp, Logger, ServiceManager } from '@foal/core';

// App
import { AppController } from './app/app.controller';
Expand All @@ -10,10 +10,13 @@ import { dataSource } from './db';
async function main() {
await dataSource.initialize();

const app = await createApp(AppController);
const serviceManager = new ServiceManager();
const logger = serviceManager.get(Logger);

const app = await createApp(AppController, { serviceManager });

const port = Config.get('port', 'number', 3001);
app.listen(port, () => displayServerURL(port));
app.listen(port, () => logger.info(`Listening on port ${port}...`));
}

main()
Expand Down
7 changes: 7 additions & 0 deletions packages/core/src/common/utils/display-server-url.util.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
/**
* Display server URL in console
*
* This function is deprecated. Use "services.get(Logger).info(`Listening on port ${port}...`)" instead.
*
* @deprecated
*/
export function displayServerURL(port: number, log = console.log) {
log();
log('\x1b[32m -------------------------------------- \x1b[0m');
Expand Down
63 changes: 2 additions & 61 deletions packages/core/src/core/config/config-not-found.error.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,65 +4,6 @@ import { strictEqual } from 'assert';
// FoalTS
import { ConfigNotFoundError } from './config-not-found.error';

const errMessage = (msg: string) => `

--------------------------------------------------------
| |
| JSON file (config/default.json, config/test.json, ...) |
| |
| -------------------------------------------------------- |
| |
| { |
| "settings": { |
| "session": { |
| "store": <your_value> |
| // OR with an environment variable: |
| "store": "env(<YOUR_ENVIRONMENT_VARIABLE>)" |
| } |
| } |
| } |
| |
--------------------------------------------------------

--------------------------------------------------------
| |
| YAML file (config/default.yml, config/test.yml, ...) |
| |
| -------------------------------------------------------- |
| |
| settings: |
| session: |
| store: <your_value> |
| # OR with an environment variable: |
| store: env(<YOUR_ENVIRONMENT_VARIABLE>) |
| |
--------------------------------------------------------

--------------------------------------------------------
| |
| JS file (config/default.js, config/test.js, ...) |
| |
| -------------------------------------------------------- |
| |
| const { Env } = require('@foal/core'); |
| |
| { |
| settings: { |
| session: { |
| store: <your_value> |
| // OR with an environment variable: |
| store: Env.get('<YOUR_ENVIRONMENT_VARIABLE>') |
| } |
| } |
| } |
| |
--------------------------------------------------------

No value found for the configuration key "settings.session.store".${msg}

To pass a value, use one of the examples above.
`;

describe('ConfigNotFoundError', () => {

it('should set the property "key" and "msg" from the constructor.', () => {
Expand All @@ -76,12 +17,12 @@ describe('ConfigNotFoundError', () => {

it('should have the proper message.', () => {
const err = new ConfigNotFoundError('settings.session.store');
strictEqual(err.message, errMessage(''));
strictEqual(err.message, `No value found for the configuration key "settings.session.store".`);
});

it('should have the proper message (custom message).', () => {
const err = new ConfigNotFoundError('settings.session.store', 'Custom message');
strictEqual(err.message, errMessage('\n\nCustom message'));
strictEqual(err.message, `No value found for the configuration key "settings.session.store". Custom message`);
});

});
88 changes: 1 addition & 87 deletions packages/core/src/core/config/config-not-found.error.ts
Original file line number Diff line number Diff line change
@@ -1,96 +1,10 @@
import { makeBox } from './utils';

export class ConfigNotFoundError extends Error {
readonly name = 'ConfigNotFoundError';

constructor(readonly key: string, readonly msg?: string) {
super();
const keywords = key.split('.');

function generateContent(type: 'JSON'|'YAML'|'JS'): string[] {
const lines: string[] = [];

if (type === 'JS') {
lines.push(' const { Env } = require(\'@foal/core\');');
lines.push('');
}

if (type !== 'YAML') {
lines.push(' {');
}

keywords.forEach((keyword, index) => {
if (type === 'JSON') {
keyword = `"${keyword}"`;
}

const spaces = ' '.repeat(index + (type === 'YAML' ? 1 : 2));

if (index === keywords.length - 1) {
lines.push(spaces + keyword + ': <your_value>');
if (type === 'YAML') {
lines.push(spaces + '# OR with an environment variable: ');
} else {
lines.push(spaces + '// OR with an environment variable: ');
}

let envValue = '';
switch (type) {
case 'JS':
envValue = 'Env.get(\'<YOUR_ENVIRONMENT_VARIABLE>\')';
break;
case 'JSON':
envValue = '"env(<YOUR_ENVIRONMENT_VARIABLE>)"';
break;
case 'YAML':
envValue = 'env(<YOUR_ENVIRONMENT_VARIABLE>)';
break;
}
lines.push(spaces + keyword + ': ' + envValue);
} else {
if (type === 'YAML') {
lines.push(spaces + keyword + ':');
} else {
lines.push(spaces + keyword + ': {');
}
}
});

if (type !== 'YAML') {
keywords.forEach((_, index) => {
if (index === keywords.length - 1) {
return;
}
const spaces = ' '.repeat(keywords.length - index);
lines.push(spaces + '}');
});

lines.push(' }');
}

return lines;
}

this.message = '\n\n'
+ makeBox(
'JSON file (config/default.json, config/test.json, ...)',
generateContent('JSON'),
)
+ '\n'
+ makeBox(
'YAML file (config/default.yml, config/test.yml, ...)',
generateContent('YAML'),
)
+ '\n'
+ makeBox(
'JS file (config/default.js, config/test.js, ...)',
generateContent('JS'),
)
+ '\n'
+ `No value found for the configuration key "${key}".\n`
+ (msg === undefined ? '' : `\n${msg}\n`)
+ '\n'
+ `To pass a value, use one of the examples above.\n`;
this.message = `No value found for the configuration key "${key}".${msg === undefined ? '' : ` ${msg}`}`;
}

}
29 changes: 5 additions & 24 deletions packages/core/src/core/config/config-type.error.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,6 @@ import { strictEqual } from 'assert';
// FoalTS
import { ConfigTypeError } from './config-type.error';

const errMessage = `

--------------------------------------------------------
| |
| Configuration file |
| |
| -------------------------------------------------------- |
| |
| { |
| settings: { |
| session: { |
|-> store: ************* |
| } |
| } |
| } |
| |
--------------------------------------------------------

The value of the configuration key "settings.session.store" has an invalid type.

Expected a "string", but got a "boolean".
`;

describe('ConfigTypeError', () => {

it('should set three properties "key", "expected" and "actual" from the constructor.', () => {
Expand All @@ -38,7 +15,11 @@ describe('ConfigTypeError', () => {

it('should have the proper message.', () => {
const err = new ConfigTypeError('settings.session.store', 'string', 'boolean');
strictEqual(err.message, errMessage);

const expected = `The value of the configuration key "settings.session.store" has an invalid type. Expected a "string", but got a "boolean".`;
const actual = err.message;

strictEqual(actual, expected);
});

});
Loading

0 comments on commit 8299833

Please sign in to comment.