Automatically abort task when client disconnects

[Prinzhorn]

I just found this plugin after already using piscina in fastify. I've decorated my request with an abortSignal:

const { AbortController } = require('abort-controller');

module.exports = function (fastify) {
  fastify.decorateRequest('abortSignal', null);

  fastify.addHook('onRequest', (req, reply, done) => {
    let controller = new AbortController();

    req.raw.once('close', () => {
      controller.abort();
    });

    req.abortSignal = controller.signal;

    done();
  });
};

So I can do this in a request handler

await piscina.runTask(task, req.abortSignal);

I'm not sure how reliable this is, but I needed to abort expensive tasks when the client cancels the fetch in the browser via AbortController (I'm using this in an Electron app). Would it make sense to integrate something like this into the plugin? The semantic is somewhat inspired by Golang context.

[Prinzhorn]

FWIW Node.js 16 has an undocumented breaking change that broke this pattern, because close fires as soon as the request has been consumed and not when the connection is closed https://github.com/nodejs/node/issues/38924

I've switched to req.raw.socket.once('close', but I assume this would break for HTTP/2, which I don't need/use in Electron. So I guess this should work, it looks like it. I do get

MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 close listeners added to [Socket]. Use emitter.setMaxListeners() to increase limit

now (Keep-Alive I guess...) :sweat_smile: . maxRequestsPerSocket: 1 doesn't make it go away.

Edit: If we ignore the MaxListenersExceededWarning it seems to work as expected. And I've moved the decorator so that it isn't applied globally. I guess I could also attach the AbortController directly to the socket with a WeakMap so that I don't keep adding listeners for the same socket.

const cache = new WeakMap();

export default function (fastify) {
  fastify.decorateRequest('abortSignal', null);

  fastify.addHook('onRequest', (req, reply, done) => {
    let socket = req.raw.socket;
    let controller = cache.get(socket);

    // Make sure we only create a single AbortController for each socket (Keep-Alive exists).
    if (!controller) {
      controller = new AbortController();
      cache.set(socket, controller);

      socket.once('close', () => {
        controller.abort();
      });
    }

    req.abortSignal = controller.signal;

    done();
  });
}

[metcoder95]

You can use fastify-racing for that, and just drop it whenever the client disconnects first (if this still an issue for you)

[Prinzhorn]

Thanks for pointing to fastify-racing! I'm happy with what I've posted above for now. I mostly opened this issue because without such a functionality I don't understand the point of fastify-piscina. It would only be logical if it's supposed to integrate piscina into fastify.

[metcoder95]

Yeah, that's what it only does, adding piscina to fastify without much 🙂