expressjs / expressjs/session

Simplify `res.end()` proxy logic — current implementation adds complexity with little real-world benefit

Open
#1,037 3 comments 1 reaction 0 assignees View on GitHub
discuss enhancement ideas
Dominant language
JavaScript
Stars
6.4k
Forks
1k
PR merge metrics
No merged PRs in 30d

Description

### Problem

The current implementation proxies `res.end()` in a way that tries to ensure the session is saved before completing the response. However, this logic is complex, fragile, and not reliable in real-world usage, particularly with redirects and concurrent browser requests.

### Background

The proxy behavior works roughly like this:

1. If data is passed to `res.end()`, it writes all but the last byte.
2. `res.write()` triggers `res.writeHead()`, buffering headers.
3. Data is buffered and sent if large enough.
4. Concurrently, `session.save()` or `session.touch()` is called.
5. The save/touch callback finally sends the last byte via `res.end()`.
6. Client receives the last packet and considers the response complete.

This seems designed to ensure session data is saved before the client makes another request. It may help in tests, but it breaks down in browsers for several reasons:

- Browsers act on redirects **immediately after receiving headers**, as redirect doesn't contain a body by the spec.
- Browsers can issue **multiple concurrent requests**, even to the same domain, which defeats the assumption of serialized communication (this was already the case in [2009](https://stackoverflow.com/a/985704)).
- With HTTP/2 and HTTP/3, multiplexed streams make the idea of strict request ordering obsolete. While those aren’t yet supported by this library (or node), they would likely require different solutions altogether.

In practice, this complexity doesn't prevent session races and makes the code in the library error prone and harder to maintain.

---

### Example Workaround

Today, to ensure a session is saved before a redirect, we have to do this manually:

```js
app.post('/login', (req, res) => {
req.session.user = 'user';
req.session.save(() => {
res.redirect('/');
});
});
```

This workaround works, but it exposes the fact that the `res.end()` proxy doesn't reliably help in the redirect case, which is likely one that users were hoping to be solved.

---

### Proposal

I propose removing the `res.end()` proxy entirely and replacing it with **much simpler logic**.

#### Option 1: Delay header send

```js
const writeHead = res.writeHead;
res.writeHead = function (...args) {
req.session.save((err) => {
if (err) throw err;
writeHead.apply(this, args);
});
};
```

This ensures that the session is saved before headers are sent. Because `res.write()` and `res.end()` call this from within their internal flow, we still need to add small proxies there, since we cannot delay execution without async/await. See #704 for some details.

As a benefit, errors can be thrown before the response is committed and thus should result 500 as the response.

#### Option 2: Async session save

```js
const end = res.end;
res.end = function (...args) {
req.session.save((err) => {
if (err) console.error(err); // Better handling should be added
});
end.apply(this, args);
};
```

This approach decouples session saving from the response entirely. While there's a risk the session isn't saved before the next request, that’s already possible today. More importantly, this avoids unnecessary complexity.

Some of the session races condition could be mitigated or logged in the store (e.g., compare and swap or diff-based updates), which are conceptually impossible in this library.

---

### Why This Matters

- The current implementation is complex, difficult to reason about, and not reliable.
- Users have problems with the redirect, as seen in #360. Also fix attempts in PR #69 and PR #157.
- Issue #477 and PR #751 show how current solution has incomplete function signature. In addition, the fix would make the code even more complex.
- Modern browsers don't behave in the way this logic expects.
- Removing this complexity would make `express-session` easier to use and maintain, while encouraging explicit and predictable session handling by developers.

---

### Conclusion

I believe we should remove the complex `res.end()` proxy logic. In addition, we should document best practices clearly (e.g., calling `session.save()` manually before redirects). I'm happy to help with an implementation or documentation update if this direction sounds good.

What do you think?

Do you find errors in my understand on reasoning?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.