Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate events from worker stream #97

Closed
ggrossetie opened this issue Aug 18, 2022 · 2 comments · Fixed by #98
Closed

Propagate events from worker stream #97

ggrossetie opened this issue Aug 18, 2022 · 2 comments · Fixed by #98

Comments

@ggrossetie
Copy link
Contributor

We've introduced a bunch of events on pino-socket but as far as I know it's not possible to listen to them as soon as we are using a ThreadStream.

Worker thread can communicate with the parent thread using require('worker_threads').parentPort.on('message'). I think it would be nice to allow transport to publish events through this port:

const parentPort = require('worker_threads').parentPort

const error = new Error('unable to write data to the TCP socket')
outputStream.emit('socketError', error)
if (parentPort) {
  parentPort.postMessage({ code: 'EVENT', name: 'socketError', args: [error] })
}

We will need to add a case in the onWorkerMessage function to propagate the event:

thread-stream/index.js

Lines 152 to 167 in deaedaa

switch (msg.code) {
case 'READY':
// Replace the FakeWeakRef with a
// proper one.
this.stream = new WeakRef(stream)
stream.flush(() => {
stream[kImpl].ready = true
stream.emit('ready')
})
break
case 'ERROR':
destroy(stream, msg.err)
break
default:
destroy(stream, new Error('this should not happen: ' + msg.code))

Something like:

switch (msg.code) { 
  case 'EVENT': 
     stream.emit(msg.name, msg.args)
     break
}

With the above changes it's possible to write:

const socketStream = pino.transport({
  target: "pino-socket",
  options: {
    address: tcpAddress,
    port: tcpPort,
    mode: "tcp",
    reconnect: true,
  }
})
// socketStream is a ThreadStream but events are propagated!
socketStream.on('socketError', (error) => {
  // do something!
})

If we don't want to propagate events but still allow transport to send post message then we should allow the code EVENT. In this case, we could write:

const socketStream = pino.transport({
  target: "pino-socket",
  options: {
    address: tcpAddress,
    port: tcpPort,
    mode: "tcp",
    reconnect: true,
  }
})
// socketStream is a ThreadStream so we can listen to messages on the worker!
socketStream.worker.on('message', (msg) => {
  // do something!
})
@mcollina
Copy link
Member

This would be nice to have. Would you like to send a PR?

@ggrossetie
Copy link
Contributor Author

Sure, I will submit a PR in the next days/weeks 👍🏻

ggrossetie added a commit to ggrossetie/thread-stream that referenced this issue Aug 22, 2022
ggrossetie added a commit to ggrossetie/thread-stream that referenced this issue Aug 22, 2022
ggrossetie added a commit to ggrossetie/thread-stream that referenced this issue Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants