Skip to content

Commit 377285d

Browse files
Mr0grogMr0grog
authoredFeb 4, 2024
Resolve circular dependency issues downstream in Winston (#207)
* Fix circular dependency between transport streams There is a circular dependency between the `TransportStream` (formerly in `index.js`) and `LegacyTransportStream` (in `legacy.js`). This causes some funky problems when you import the legacy module and later import the main module, as seen in winstonjs/winston#1886. I've resolved the circular issue here by definiting the main implementation of `TransportStream` in a separate module (which `LegacyTransportStream` now depends on) and then doing the work of decorating it with a `.LegacyTransportStream` property in `index.js` (and that's pretty much all the index module does). * Resolve lint warning while I'm here
·
v4.9.0v4.7.0
1 parent 66e8fdb commit 377285d

File tree

4 files changed

+215
-212
lines changed

4 files changed

+215
-212
lines changed
 

‎index.js

+2-210
Original file line numberDiff line numberDiff line change
@@ -1,215 +1,7 @@
11
'use strict';
22

3-
const util = require('util');
4-
const Writable = require('readable-stream/lib/_stream_writable.js');
5-
const { LEVEL } = require('triple-beam');
6-
7-
/**
8-
* Constructor function for the TransportStream. This is the base prototype
9-
* that all `winston >= 3` transports should inherit from.
10-
* @param {Object} options - Options for this TransportStream instance
11-
* @param {String} options.level - Highest level according to RFC5424.
12-
* @param {Boolean} options.handleExceptions - If true, info with
13-
* { exception: true } will be written.
14-
* @param {Function} options.log - Custom log function for simple Transport
15-
* creation
16-
* @param {Function} options.close - Called on "unpipe" from parent.
17-
*/
18-
const TransportStream = module.exports = function TransportStream(options = {}) {
19-
Writable.call(this, { objectMode: true, highWaterMark: options.highWaterMark });
20-
21-
this.format = options.format;
22-
this.level = options.level;
23-
this.handleExceptions = options.handleExceptions;
24-
this.handleRejections = options.handleRejections;
25-
this.silent = options.silent;
26-
27-
if (options.log) this.log = options.log;
28-
if (options.logv) this.logv = options.logv;
29-
if (options.close) this.close = options.close;
30-
31-
// Get the levels from the source we are piped from.
32-
this.once('pipe', logger => {
33-
// Remark (indexzero): this bookkeeping can only support multiple
34-
// Logger parents with the same `levels`. This comes into play in
35-
// the `winston.Container` code in which `container.add` takes
36-
// a fully realized set of options with pre-constructed TransportStreams.
37-
this.levels = logger.levels;
38-
this.parent = logger;
39-
});
40-
41-
// If and/or when the transport is removed from this instance
42-
this.once('unpipe', src => {
43-
// Remark (indexzero): this bookkeeping can only support multiple
44-
// Logger parents with the same `levels`. This comes into play in
45-
// the `winston.Container` code in which `container.add` takes
46-
// a fully realized set of options with pre-constructed TransportStreams.
47-
if (src === this.parent) {
48-
this.parent = null;
49-
if (this.close) {
50-
this.close();
51-
}
52-
}
53-
});
54-
};
55-
56-
/*
57-
* Inherit from Writeable using Node.js built-ins
58-
*/
59-
util.inherits(TransportStream, Writable);
60-
61-
/**
62-
* Writes the info object to our transport instance.
63-
* @param {mixed} info - TODO: add param description.
64-
* @param {mixed} enc - TODO: add param description.
65-
* @param {function} callback - TODO: add param description.
66-
* @returns {undefined}
67-
* @private
68-
*/
69-
TransportStream.prototype._write = function _write(info, enc, callback) {
70-
if (this.silent || (info.exception === true && !this.handleExceptions)) {
71-
return callback(null);
72-
}
73-
74-
// Remark: This has to be handled in the base transport now because we
75-
// cannot conditionally write to our pipe targets as stream. We always
76-
// prefer any explicit level set on the Transport itself falling back to
77-
// any level set on the parent.
78-
const level = this.level || (this.parent && this.parent.level);
79-
80-
if (!level || this.levels[level] >= this.levels[info[LEVEL]]) {
81-
if (info && !this.format) {
82-
return this.log(info, callback);
83-
}
84-
85-
let errState;
86-
let transformed;
87-
88-
// We trap(and re-throw) any errors generated by the user-provided format, but also
89-
// guarantee that the streams callback is invoked so that we can continue flowing.
90-
try {
91-
transformed = this.format.transform(Object.assign({}, info), this.format.options);
92-
} catch (err) {
93-
errState = err;
94-
}
95-
96-
if (errState || !transformed) {
97-
// eslint-disable-next-line callback-return
98-
callback();
99-
if (errState) throw errState;
100-
return;
101-
}
102-
103-
return this.log(transformed, callback);
104-
}
105-
this._writableState.sync = false;
106-
return callback(null);
107-
};
108-
109-
/**
110-
* Writes the batch of info objects (i.e. "object chunks") to our transport
111-
* instance after performing any necessary filtering.
112-
* @param {mixed} chunks - TODO: add params description.
113-
* @param {function} callback - TODO: add params description.
114-
* @returns {mixed} - TODO: add returns description.
115-
* @private
116-
*/
117-
TransportStream.prototype._writev = function _writev(chunks, callback) {
118-
if (this.logv) {
119-
const infos = chunks.filter(this._accept, this);
120-
if (!infos.length) {
121-
return callback(null);
122-
}
123-
124-
// Remark (indexzero): from a performance perspective if Transport
125-
// implementers do choose to implement logv should we make it their
126-
// responsibility to invoke their format?
127-
return this.logv(infos, callback);
128-
}
129-
130-
for (let i = 0; i < chunks.length; i++) {
131-
if (!this._accept(chunks[i])) continue;
132-
133-
if (chunks[i].chunk && !this.format) {
134-
this.log(chunks[i].chunk, chunks[i].callback);
135-
continue;
136-
}
137-
138-
let errState;
139-
let transformed;
140-
141-
// We trap(and re-throw) any errors generated by the user-provided format, but also
142-
// guarantee that the streams callback is invoked so that we can continue flowing.
143-
try {
144-
transformed = this.format.transform(
145-
Object.assign({}, chunks[i].chunk),
146-
this.format.options
147-
);
148-
} catch (err) {
149-
errState = err;
150-
}
151-
152-
if (errState || !transformed) {
153-
// eslint-disable-next-line callback-return
154-
chunks[i].callback();
155-
if (errState) {
156-
// eslint-disable-next-line callback-return
157-
callback(null);
158-
throw errState;
159-
}
160-
} else {
161-
this.log(transformed, chunks[i].callback);
162-
}
163-
}
164-
165-
return callback(null);
166-
};
167-
168-
/**
169-
* Predicate function that returns true if the specfied `info` on the
170-
* WriteReq, `write`, should be passed down into the derived
171-
* TransportStream's I/O via `.log(info, callback)`.
172-
* @param {WriteReq} write - winston@3 Node.js WriteReq for the `info` object
173-
* representing the log message.
174-
* @returns {Boolean} - Value indicating if the `write` should be accepted &
175-
* logged.
176-
*/
177-
TransportStream.prototype._accept = function _accept(write) {
178-
const info = write.chunk;
179-
if (this.silent) {
180-
return false;
181-
}
182-
183-
// We always prefer any explicit level set on the Transport itself
184-
// falling back to any level set on the parent.
185-
const level = this.level || (this.parent && this.parent.level);
186-
187-
// Immediately check the average case: log level filtering.
188-
if (
189-
info.exception === true ||
190-
!level ||
191-
this.levels[level] >= this.levels[info[LEVEL]]
192-
) {
193-
// Ensure the info object is valid based on `{ exception }`:
194-
// 1. { handleExceptions: true }: all `info` objects are valid
195-
// 2. { exception: false }: accepted by all transports.
196-
if (this.handleExceptions || info.exception !== true) {
197-
return true;
198-
}
199-
}
200-
201-
return false;
202-
};
203-
204-
/**
205-
* _nop is short for "No operation"
206-
* @returns {Boolean} Intentionally false.
207-
*/
208-
TransportStream.prototype._nop = function _nop() {
209-
// eslint-disable-next-line no-undefined
210-
return void undefined;
211-
};
212-
3+
// Expose modern transport directly as the export
4+
module.exports = require('./modern');
2135

2146
// Expose legacy stream
2157
module.exports.LegacyTransportStream = require('./legacy');

‎legacy.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
const util = require('util');
44
const { LEVEL } = require('triple-beam');
5-
const TransportStream = require('./');
5+
const TransportStream = require('./modern');
66

77
/**
88
* Constructor function for the LegacyTransportStream. This is an internal

‎modern.js

+211
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
'use strict';
2+
3+
const util = require('util');
4+
const Writable = require('readable-stream/lib/_stream_writable.js');
5+
const { LEVEL } = require('triple-beam');
6+
7+
/**
8+
* Constructor function for the TransportStream. This is the base prototype
9+
* that all `winston >= 3` transports should inherit from.
10+
* @param {Object} options - Options for this TransportStream instance
11+
* @param {String} options.level - Highest level according to RFC5424.
12+
* @param {Boolean} options.handleExceptions - If true, info with
13+
* { exception: true } will be written.
14+
* @param {Function} options.log - Custom log function for simple Transport
15+
* creation
16+
* @param {Function} options.close - Called on "unpipe" from parent.
17+
*/
18+
const TransportStream = module.exports = function TransportStream(options = {}) {
19+
Writable.call(this, { objectMode: true, highWaterMark: options.highWaterMark });
20+
21+
this.format = options.format;
22+
this.level = options.level;
23+
this.handleExceptions = options.handleExceptions;
24+
this.handleRejections = options.handleRejections;
25+
this.silent = options.silent;
26+
27+
if (options.log) this.log = options.log;
28+
if (options.logv) this.logv = options.logv;
29+
if (options.close) this.close = options.close;
30+
31+
// Get the levels from the source we are piped from.
32+
this.once('pipe', logger => {
33+
// Remark (indexzero): this bookkeeping can only support multiple
34+
// Logger parents with the same `levels`. This comes into play in
35+
// the `winston.Container` code in which `container.add` takes
36+
// a fully realized set of options with pre-constructed TransportStreams.
37+
this.levels = logger.levels;
38+
this.parent = logger;
39+
});
40+
41+
// If and/or when the transport is removed from this instance
42+
this.once('unpipe', src => {
43+
// Remark (indexzero): this bookkeeping can only support multiple
44+
// Logger parents with the same `levels`. This comes into play in
45+
// the `winston.Container` code in which `container.add` takes
46+
// a fully realized set of options with pre-constructed TransportStreams.
47+
if (src === this.parent) {
48+
this.parent = null;
49+
if (this.close) {
50+
this.close();
51+
}
52+
}
53+
});
54+
};
55+
56+
/*
57+
* Inherit from Writeable using Node.js built-ins
58+
*/
59+
util.inherits(TransportStream, Writable);
60+
61+
/**
62+
* Writes the info object to our transport instance.
63+
* @param {mixed} info - TODO: add param description.
64+
* @param {mixed} enc - TODO: add param description.
65+
* @param {function} callback - TODO: add param description.
66+
* @returns {undefined}
67+
* @private
68+
*/
69+
TransportStream.prototype._write = function _write(info, enc, callback) {
70+
if (this.silent || (info.exception === true && !this.handleExceptions)) {
71+
return callback(null);
72+
}
73+
74+
// Remark: This has to be handled in the base transport now because we
75+
// cannot conditionally write to our pipe targets as stream. We always
76+
// prefer any explicit level set on the Transport itself falling back to
77+
// any level set on the parent.
78+
const level = this.level || (this.parent && this.parent.level);
79+
80+
if (!level || this.levels[level] >= this.levels[info[LEVEL]]) {
81+
if (info && !this.format) {
82+
return this.log(info, callback);
83+
}
84+
85+
let errState;
86+
let transformed;
87+
88+
// We trap(and re-throw) any errors generated by the user-provided format, but also
89+
// guarantee that the streams callback is invoked so that we can continue flowing.
90+
try {
91+
transformed = this.format.transform(Object.assign({}, info), this.format.options);
92+
} catch (err) {
93+
errState = err;
94+
}
95+
96+
if (errState || !transformed) {
97+
// eslint-disable-next-line callback-return
98+
callback();
99+
if (errState) throw errState;
100+
return;
101+
}
102+
103+
return this.log(transformed, callback);
104+
}
105+
this._writableState.sync = false;
106+
return callback(null);
107+
};
108+
109+
/**
110+
* Writes the batch of info objects (i.e. "object chunks") to our transport
111+
* instance after performing any necessary filtering.
112+
* @param {mixed} chunks - TODO: add params description.
113+
* @param {function} callback - TODO: add params description.
114+
* @returns {mixed} - TODO: add returns description.
115+
* @private
116+
*/
117+
TransportStream.prototype._writev = function _writev(chunks, callback) {
118+
if (this.logv) {
119+
const infos = chunks.filter(this._accept, this);
120+
if (!infos.length) {
121+
return callback(null);
122+
}
123+
124+
// Remark (indexzero): from a performance perspective if Transport
125+
// implementers do choose to implement logv should we make it their
126+
// responsibility to invoke their format?
127+
return this.logv(infos, callback);
128+
}
129+
130+
for (let i = 0; i < chunks.length; i++) {
131+
if (!this._accept(chunks[i])) continue;
132+
133+
if (chunks[i].chunk && !this.format) {
134+
this.log(chunks[i].chunk, chunks[i].callback);
135+
continue;
136+
}
137+
138+
let errState;
139+
let transformed;
140+
141+
// We trap(and re-throw) any errors generated by the user-provided format, but also
142+
// guarantee that the streams callback is invoked so that we can continue flowing.
143+
try {
144+
transformed = this.format.transform(
145+
Object.assign({}, chunks[i].chunk),
146+
this.format.options
147+
);
148+
} catch (err) {
149+
errState = err;
150+
}
151+
152+
if (errState || !transformed) {
153+
// eslint-disable-next-line callback-return
154+
chunks[i].callback();
155+
if (errState) {
156+
// eslint-disable-next-line callback-return
157+
callback(null);
158+
throw errState;
159+
}
160+
} else {
161+
this.log(transformed, chunks[i].callback);
162+
}
163+
}
164+
165+
return callback(null);
166+
};
167+
168+
/**
169+
* Predicate function that returns true if the specfied `info` on the
170+
* WriteReq, `write`, should be passed down into the derived
171+
* TransportStream's I/O via `.log(info, callback)`.
172+
* @param {WriteReq} write - winston@3 Node.js WriteReq for the `info` object
173+
* representing the log message.
174+
* @returns {Boolean} - Value indicating if the `write` should be accepted &
175+
* logged.
176+
*/
177+
TransportStream.prototype._accept = function _accept(write) {
178+
const info = write.chunk;
179+
if (this.silent) {
180+
return false;
181+
}
182+
183+
// We always prefer any explicit level set on the Transport itself
184+
// falling back to any level set on the parent.
185+
const level = this.level || (this.parent && this.parent.level);
186+
187+
// Immediately check the average case: log level filtering.
188+
if (
189+
info.exception === true ||
190+
!level ||
191+
this.levels[level] >= this.levels[info[LEVEL]]
192+
) {
193+
// Ensure the info object is valid based on `{ exception }`:
194+
// 1. { handleExceptions: true }: all `info` objects are valid
195+
// 2. { exception: false }: accepted by all transports.
196+
if (this.handleExceptions || info.exception !== true) {
197+
return true;
198+
}
199+
}
200+
201+
return false;
202+
};
203+
204+
/**
205+
* _nop is short for "No operation"
206+
* @returns {Boolean} Intentionally false.
207+
*/
208+
TransportStream.prototype._nop = function _nop() {
209+
// eslint-disable-next-line no-undefined
210+
return void undefined;
211+
};

‎test/index.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ function infoify(info) {
2424
info[LEVEL] = info.level;
2525
info[MESSAGE] = info.message;
2626
return info;
27-
};
27+
}
2828

2929
describe('TransportStream', () => {
3030
it('should have the appropriate methods defined', () => {

0 commit comments

Comments
 (0)
Please sign in to comment.