Async session stores don't work with newer versions of nodejs

mrbrianevans commented 1 month ago

Prerequisites

[X] I have written a descriptive issue title
[X] I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.27.0

Plugin version

10.8.0

Node.js version

20.x

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

docker

Description

This was reported in https://github.com/fastify/session/issues/252 but the issue was closed due to a particular implementation flaw by the poster. However the problem seems more widespread than just his issue with handling errors.

Asynchronous methods on the session store (which is required by almost any production session store to persist in a database) cause an error with node versions starting at node 20. Works fine with node 19.

As a minimum repoducible example, I've narrowed it down to adding a simple setTimeout in the default in-memory store causes the bug to occur.

import {EventEmitter} from 'node:events'
import util from 'node:util'
+ import { setTimeout } from "timers/promises";

function Store (storeMap = new Map()) {
  this.store = storeMap
  EventEmitter.call(this)
}

util.inherits(Store, EventEmitter)

Store.prototype.set = async function set (sessionId, session, callback) {
  this.store.set(sessionId, session)
+  await setTimeout(50)
  callback()
}

Store.prototype.get = function get (sessionId, callback) {
  const session = this.store.get(sessionId)
  callback(null, session)
}

Store.prototype.destroy = function destroy (sessionId, callback) {
  this.store.delete(sessionId)
  callback()
}

export {Store}

When using this store in a fastify app such as (simplified)

const fastify = Fastify({})
await fastify.register(import('@fastify/session'), {
  ...
  store: new Store()
})

It throws this error and exits the process

Error [ERR_HTTP_HEADERS_SENT]: Cannot write headers after they are sent to the client

This is on Node.js v20.13.1 but it affects any version of node since 20.x. Previously known working version was node 19.

Link to code that reproduces the bug

No response

Expected Behavior

I expect it to work on node 20 the same way it did on node 19 or prior.

Any sensible session store requires some asynchronous operation to set and get, so if my diagnosis is correct, this is a major issue.

mcollina commented 1 month ago

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

phal0r commented 5 hours ago

We are facing the same problem. It is hard to narrow down, but imho a critical bug.

fastify / session