Node: Adding middleware functions in EventEmitter .on()

Created on 17 Jun 2020  ·  10Comments  ·  Source: nodejs/node

Is your feature request related to a problem? Please describe.
In SocketIO it would be useful to be able to add middleware functions to specific events. The sockets use the EventEmitter.

Describe the solution you'd like
In a regular express app it is possible to add middleware to specific endpoints like so:

function someFunc(req, res, next) {
    // doing something
   next()
}
app.get('/users', someFunc, (req, res) => { res.send('someData')})

It would be logical to also be able to do it with SocketIO:

function someFunc(data, next) {
    // doing something
   next()
}
function someFunc2(data, next) {
    // doing something
   next()
}
socket.on('users', someFunc, someFunc2, (data) => { /*doing something*/})

Why it would be used
This could be useful to add callbacks to endpoints to check if a user is authenticated or has enough credits before using the endpoint for example. Instead of having a global middleware that is checked for every endpoint.

So by implementing these extra middleware to be added in the EventEmitter.on() function it could have the same behavior as the express routes and still maintain the same behavior with just one function.

Describe alternatives you've considered

  • using the socket.use(fn) function would bring a lot of overhead as it would call the middleware every time for every endpoint.
    (added: )
  • it could also be a SocketIO feature, though I think it could also be useful for other applications where specific handling per event endpoint is needed.
  • It could be possible to create a wrapper function like socket.on('users', functionWrapper( fn1, fn2, fn3)) though it would be cleaner to be able to directly put the functions in the .on() function.

example + further implementation/usage info
JSFIDDLE
comment below

events feature request

Most helpful comment

I'm generally -1 on this for a couple of reasons.

1) It's something that is better implemented in userland. There are examples of generalized middleware modules that can be used to implement this kind of behavior: https://github.com/fastify/middie

2) It would have a non-trivial impact on EventEmitter performance.

3) It would expand the functionality discrepancy between EventEmitter and EventTarget

All 10 comments

Not sure how much of an impact this change will have on EventEmitter performance, I can try this out.

Hi @Dirvann, this seems like an interesting request, although I'm not entirely sure Node.js is the appropriate place to implement it (but I'll defer to folks who are more familiar with EventEmitter, cc @nodejs/events). Have you considered adding this as a feature of SocketIO instead of Node.js (by wrapping or inhereting EventEmitter, for example)?

Also, please in the future follow the issues template, it helps us better understand why a request is necessary as well as alternatives considered (which is missing in this request).

@mmarchini I followed the logical structure of the templates, though didn't add the titles added them now.

For a temporary solution i'm trying to make something like wrapping all the functions in the list into one. Something like:
socket.on('users', functionWrapper( fn1, fn2, fn3))

Though I think implementing multiple functions directly in the EventEmitter would me cleaner and also useful for other applications, where specific endpoints of events would need special handling.

We can use the return value of the function as the parameter of the next function, if the return value is undefined will not trigger the next function.

e.g.
`js function someFunc(count) { // doing something return count; } function someFunc2(count) { // doing something return count + 2; } socket.on('users', someFunc, someFunc2, (count) => { /*doing something*/})

Hey, thank you for reaching out and opening this issue. I don't understand this feature request, could you add some more coding examples?

Is your request for there to be some way for events to block the execution of further event handlers until some async action has completed?

If that's the case then I think it's very challenging to do in a way that does not violate invariants and expectations people have regarding event dispatch.

@benjamingr It could defenitely be a funtionality if you want an async task to happen in the middleware, or even to stop you from running the last middleware for a security implementation for example.

I have a small working demo here where i've implemented the functionality. It isn't that intrusive as it only adds a wrapper for the list of functions given. So in the end there is still just one function per listener. Here is the demo on JSFIDDLE

new emitter class

/**
* returns a function with parameters as input that 
* runs all the given functions sequentially
* @return function(...funcParams)
*/
function funcWrapper(...funcs) {
  function exec(s,fs, ...dat) {
    s< fs.length - 1?
    fs[s](...dat, () => exec(++s, fs, ...dat)):
    fs[s](...dat)
  }
  return exec.bind(null,0, funcs)
}

/**
* simulation of a class extending EventEmitter
*/
class emitterClass {
  constructor() {
    this.listeners = new Map()
  }

  /**
  * add listener with multiple functions
  */
  on(name, ...func) {
    if(!this.listeners.has(name)) this.listeners.set(name, [])

    // create a wrapper function from all the functions
    let f  = funcWrapper(...func)
    this.listeners.get(name).push(f)
  }

  /**
  * emit function is unchanged from the original
  */
  emit(name, ...data) {
    if(!this.listeners.has(name)) return
    this.listeners.get(name).forEach(func => func(...data))
  }
}

USAGE

regular behavior still the same

let l = new emitterClass()
l.on('users', (data) => console.log('userdata:',data))
l.emit('users', {name: 'mark'})
// "userdata: { name: 'mark' }"

regular multiple parameters

let s = new emitterClass()
s.on('users', (data, data2) => {
  console.log('userdata:',data, 'second: ', data2)
})
s.emit('users', {name: 'mark'}, {name: 'josh'})
// "userdata: { name: 'mark' } second:  { name: 'josh' }"

multiple middleware functions

function addAge(data, next) {
  if(data.name === 'mark') {
    data.age = 45
  }
  next()
}

function addJob(data, next) {
  // some database information for example
  if(data.name === 'mark') {
    data.job = 'jobless'
  }
  next()
}

let r = new emitterClass()
r.on('users', addAge, addJob, console.log)
r.emit('users', {name:'mark'})
// "{ name: 'mark', age: 45, job: 'jobless' }"

multiple middleware functions + multiple params

function addAge2(data,  data2, next) {
  if(data.name === 'mark') {
    data.age = 45
  }
  data2.age = 24
  next()
}

function addJob2(data, data2, next) {
  // some database information for example
  if(data.name === 'mark') {
    data.job = 'jobless'
  }
  if(data2.name === 'lisa') {
    data2.job = 'carpenter'
  }
  next()
}

let t = new emitterClass()
t.on('users', addAge2, addJob2, console.log)
t.emit('users', {name:'mark'}, {name:'lisa'})
// "{ name: 'mark', age: 45, job: 'jobless' } { name: 'lisa', age: 24, job: 'carpenter' }"

block next functions from running

function hasAccess(data, next) {
  if(data.name === 'mark' && data.pass === 1234) {
    return next()
  }
  console.log('WRONG PASSWORD')
}

function sensitiveInfo(data, next) {
  // some database information for example
  console.log('government password')
  next()
}

let e = new emitterClass()
e.on('users', hasAccess, sensitiveInfo, console.log)
e.emit('users', {name:'mark', pass:1234})
// "government password  { name: 'mark', pass: 1234 }"
e.emit('users', {name:'mark', pass:123})
// "WRONG PASSWORD"

#### SocketIO usage example
In SocketIO there is also a function to return data to the requester like
socket.on('getData', (data, callback) => callback('secretPasswords'))

In a situation where for specific endpoints you can only access data if you have a specific state/credential, it would be useful to just add a checker middleware like so:

let database= {pass: 1234}

function hasAccess(data, callback, next) {
     if(data.pass === database.pass){
         return next()
      }
      callback('you do not have access')
}

socket.on('secretData', hasAccess, (data, callback) => callback('secretPassword'))
socket.on('normalData', (data, callback) => callback('normalData'))

Further on implementation

Some function will be slightly changed. the .on() function must be slightly modified to allow a list of functions. And can bundle them into one. Also the removeListener() and .once().

I'm generally -1 on this for a couple of reasons.

1) It's something that is better implemented in userland. There are examples of generalized middleware modules that can be used to implement this kind of behavior: https://github.com/fastify/middie

2) It would have a non-trivial impact on EventEmitter performance.

3) It would expand the functionality discrepancy between EventEmitter and EventTarget

@jasnell Thanks for the response.

  1. It's definitely be implemented using another module. I looked into middie although it isn't exactly how I'd prefer it to be. But it is not really a question of creating a module, I would think this is a fairly simple feature to be used everywhere. In the end for the user it's just being able to add some extra functions.

  2. The performance isn't reduced. I have implemented it in this commit. When a single function is given as listener. It adds the function as listener. Just as usual. So no extra array reads needed.
    Then when multiple functions are given, an extra they will be cycled through with a function. In a regular application an extreme example could have like 10 middleware functions it would need 10 array accesses containing the functions.

// wrap a list of functions into one
function functionWrapper(...functions) {
 if(functions.length === 1) return functions[0]
  function exec(s,fs, ...params) {
    s< fs.length - 1?
    fs[s](...params, () => exec(++s, fs, ...params)):
    fs[s](...params);
  }
  return exec.bind(null,0, functions);
}
  1. Is it a problem if EventTargetand EventEmitterhave more discrepancy? And if so, then adding the feature to EventTarget also would solve that issue right?

I've started implementing here as I stated before and already integrated the necessary changes to the whole file (not tested yet). I'll work a little more on the formatting, error handling and tests to put it in the right pull request format.

This feature very looks like stream.pipeline. what if we expose an API called require(‘events’).pipeline?

Hey, it looks like discussion here has exhausted itself. I recommend this is first explored through a userland implementation so it's easier for maintainers and the community to play with and get a feel of. Personally I agree with what James wrote above.

I am going to go ahead and close this _but_ if you believe there is more discussion to be had please let me know and I will gladly reopen this.

I know this suggestion didn't come to fruition (yet) - but I appreciate it, so thanks 🙏

Was this page helpful?
0 / 5 - 0 ratings

Related issues

sandeepks1 picture sandeepks1  ·  3Comments

loretoparisi picture loretoparisi  ·  3Comments

fanjunzhi picture fanjunzhi  ·  3Comments

filipesilvaa picture filipesilvaa  ·  3Comments

danielstaleiny picture danielstaleiny  ·  3Comments