Skip to content

Commit 44358a8

Browse files
committed
Fix log message doesn't appearing correctly in the Bolt log.
The write and receive message functions were moved from the connection to the `response-handler` and `bolt-protocol`, but logs on the functions still creating the message using the `this`, but in the nnew context the value of `this` is not serialized as the connection id. Since the connection id data is dynamic and the depends on a response from the server, the issue was solved decorating the logger function to add connection context information.
1 parent 0442de0 commit 44358a8

File tree

4 files changed

+57
-16
lines changed

4 files changed

+57
-16
lines changed

bolt-connection/src/bolt/bolt-protocol-v1.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ export default class BoltProtocol {
334334

335335
if (queued) {
336336
if (this._log.isDebugEnabled()) {
337-
this._log.debug(`${this} C: ${message}`)
337+
this._log.debug(`C: ${message}`)
338338
}
339339

340340
this.packer().packStruct(

bolt-connection/src/bolt/response-handler.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,13 @@ export default class ResponseHandler {
9292
switch (msg.signature) {
9393
case RECORD:
9494
if (this._log.isDebugEnabled()) {
95-
this._log.debug(`${this} S: RECORD ${json.stringify(msg)}`)
95+
this._log.debug(`S: RECORD ${json.stringify(msg)}`)
9696
}
9797
this._currentObserver.onNext(payload)
9898
break
9999
case SUCCESS:
100100
if (this._log.isDebugEnabled()) {
101-
this._log.debug(`${this} S: SUCCESS ${json.stringify(msg)}`)
101+
this._log.debug(`S: SUCCESS ${json.stringify(msg)}`)
102102
}
103103
try {
104104
const metadata = this._transformMetadata(payload)
@@ -109,7 +109,7 @@ export default class ResponseHandler {
109109
break
110110
case FAILURE:
111111
if (this._log.isDebugEnabled()) {
112-
this._log.debug(`${this} S: FAILURE ${json.stringify(msg)}`)
112+
this._log.debug(`S: FAILURE ${json.stringify(msg)}`)
113113
}
114114
try {
115115
const error = newError(payload.message, payload.code)
@@ -125,7 +125,7 @@ export default class ResponseHandler {
125125
break
126126
case IGNORED:
127127
if (this._log.isDebugEnabled()) {
128-
this._log.debug(`${this} S: IGNORED ${json.stringify(msg)}`)
128+
this._log.debug(`S: IGNORED ${json.stringify(msg)}`)
129129
}
130130
try {
131131
if (this._currentFailure && this._currentObserver.onError) {

bolt-connection/src/connection/connection-channel.js

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,14 @@
1818
*/
1919

2020
import { Chunker, Dechunker, ChannelConfig, Channel } from '../channel'
21-
import { newError, error, json } from 'neo4j-driver-core'
21+
import { newError, error, json, internal } from 'neo4j-driver-core'
2222
import Connection from './connection'
2323
import Bolt from '../bolt'
2424

2525
const { PROTOCOL_ERROR } = error
26+
const {
27+
logger: { Logger }
28+
} = internal
2629

2730
let idGenerator = 0
2831

@@ -57,15 +60,14 @@ export function createChannelConnection (
5760
const createProtocol = conn =>
5861
Bolt.create({
5962
version,
60-
connection: conn,
6163
channel,
6264
chunker,
6365
dechunker,
6466
disableLosslessIntegers: config.disableLosslessIntegers,
6567
useBigInt: config.useBigInt,
6668
serversideRouting,
6769
server: conn.server,
68-
log,
70+
log: conn.logger,
6971
observer: {
7072
onError: conn._handleFatalError.bind(conn),
7173
onFailure: conn._resetOnFailure.bind(conn),
@@ -97,7 +99,6 @@ export function createChannelConnection (
9799
})
98100
)
99101
}
100-
101102
export default class ChannelConnection extends Connection {
102103
/**
103104
* @constructor
@@ -128,7 +129,7 @@ export default class ChannelConnection extends Connection {
128129
this._disableLosslessIntegers = disableLosslessIntegers
129130
this._ch = channel
130131
this._chunker = chunker
131-
this._log = log
132+
this._log = createConnectionLogger(this, log)
132133
this._serversideRouting = serversideRouting
133134

134135
// connection from the database, returned in response for HELLO message and might not be available
@@ -145,7 +146,7 @@ export default class ChannelConnection extends Connection {
145146
this._isBroken = false
146147

147148
if (this._log.isDebugEnabled()) {
148-
this._log.debug(`${this} created towards ${address}`)
149+
this._log.debug(`created towards ${address}`)
149150
}
150151
}
151152

@@ -234,6 +235,10 @@ export default class ChannelConnection extends Connection {
234235
return this._server
235236
}
236237

238+
get logger () {
239+
return this._log
240+
}
241+
237242
/**
238243
* "Fatal" means the connection is dead. Only call this if something
239244
* happens that cannot be recovered from. This will lead to all subscribers
@@ -247,7 +252,7 @@ export default class ChannelConnection extends Connection {
247252

248253
if (this._log.isErrorEnabled()) {
249254
this._log.error(
250-
`${this} experienced a fatal error ${json.stringify(this._error)}`
255+
`experienced a fatal error ${json.stringify(this._error)}`
251256
)
252257
}
253258

@@ -322,7 +327,7 @@ export default class ChannelConnection extends Connection {
322327
*/
323328
async close () {
324329
if (this._log.isDebugEnabled()) {
325-
this._log.debug(`${this} closing`)
330+
this._log.debug('closing')
326331
}
327332

328333
if (this._protocol && this.isOpen()) {
@@ -334,7 +339,7 @@ export default class ChannelConnection extends Connection {
334339
await this._ch.close()
335340

336341
if (this._log.isDebugEnabled()) {
337-
this._log.debug(`${this} closed`)
342+
this._log.debug('closed')
338343
}
339344
}
340345

@@ -350,3 +355,15 @@ export default class ChannelConnection extends Connection {
350355
return error
351356
}
352357
}
358+
359+
/**
360+
* Creates a log with the connection info as prefix
361+
* @param {Connection} connection The connection
362+
* @param {Logger} logger The logger
363+
* @returns {Logger} The new logger with enriched messages
364+
*/
365+
function createConnectionLogger (connection, logger) {
366+
return new Logger(logger._level, (level, message) =>
367+
logger._loggerFunction(level, `${connection} ${message}`)
368+
)
369+
}

test/internal/connection-channel.test.js

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,13 +543,19 @@ describe('#integration ChannelConnection', () => {
543543
/**
544544
* @return {Promise<Connection>}
545545
*/
546-
function createConnection (url, config, errorCode = null) {
546+
function createConnection (
547+
url,
548+
config,
549+
errorCode = null,
550+
logger = createVerifyConnectionIdLogger()
551+
) {
547552
const _config = config || {}
553+
connection = undefined
548554
return createChannelConnection(
549555
ServerAddress.fromUrl(url),
550556
_config,
551557
new ConnectionErrorHandler(errorCode || SERVICE_UNAVAILABLE),
552-
Logger.noOp()
558+
logger
553559
).then(c => {
554560
connection = c
555561
return connection
@@ -563,4 +569,22 @@ describe('#integration ChannelConnection', () => {
563569
originalWrite(message, observer, flush)
564570
}
565571
}
572+
573+
function createVerifyConnectionIdLogger () {
574+
return new Logger('debug', (_, message) => {
575+
if (!connection) {
576+
// the connection is not the context, so we could
577+
// only assert if it starts with Connection [
578+
expect(
579+
message.startsWith('Connection ['),
580+
`Log message "${message}" should starts with "Connection ["`
581+
).toBe(true)
582+
return
583+
}
584+
expect(
585+
message.startsWith(`${connection}`),
586+
`Log message "${message}" should starts with "${connection}"`
587+
).toBe(true)
588+
})
589+
}
566590
})

0 commit comments

Comments
 (0)