Skip to content

Conversation

@yanxue-22
Copy link
Contributor

@yanxue-22 yanxue-22 commented Oct 17, 2025

Problem

When Express middleware modified req (e.g., req.body, req.query, req.params, req.headers), those modifications were not passed to route handlers defined without the routeHandler() wrapper.

More confidence on why this is real bug:

  • Function has returned just input since 2022 - never merged req modifications
  • Function name is "IgnoringResults" - should ignore return values, not req modifications
  • Original commit (0c311dd) says "allow middleware to add request properties" but implementation doesn't

Root Cause

  1. req.decoded is created once before middleware runs
  2. The route.request.decode() function uses io-ts validation, which creates a new object from req.body, req.query, req.params, and req.headers
  3. Middleware runs and modifies the Express request object properties (e.g., req.body.someField = 'value')
  4. runMiddlewareChainIgnoringResults() returned only the original req.decoded, ignoring any middleware modifications
  5. Handler receives outdated params without middleware changes

Example of Broken Behavior

const modifyBodyMiddleware: express.RequestHandler = (req, _res, next) => {
req.body.addedByMiddleware = 'ADDED'; // ❌ This modification was lost
next();
};

const handler = async (params) => {
console.log(params.addedByMiddleware); // undefined (should be 'ADDED')
};

Solution

Modified runMiddlewareChainIgnoringResults() in packages/express-wrapper/src/middleware.ts to merge middleware modifications back into the handler params.

Before

return input; // Just returned original decoded input

After

return {
  ...(req as any).decoded,   // Start with original decoded request
  ...(req.body || {}),        // Merge any body modifications
  ...(req.query || {}),       // Merge any query modifications
  ...(req.params || {}),      // Merge any params modifications
  ...(req.headers || {})      // Merge any headers modifications
};

This ensures that after middleware runs, any modifications to the four sources of req.decoded are merged back into the result passed to the handler.


Why merge req.body, req.query, req.params, and req.headers specifically?

These four properties are hard-coded because they are the exact sources used to build req.decoded:

const decoded = route.request.decode({
body: req.body,
headers: req.headers,
params: req.params,
query: req.query,
});

Since route.request.decode() creates a new object from these four sources, we must merge them back after middleware runs to capture any modifications.


Closes #188

@yanxue-22 yanxue-22 force-pushed the DX-1301-Fix-Lost-Request-Data-Using-Middleware branch from bd8ee11 to b3c6bb1 Compare October 17, 2025 20:28
@yanxue-22 yanxue-22 changed the title test: add test for middleware that modifies req object fix: verify route-level middleware req.body modifications reach handler Oct 17, 2025
@yanxue-22 yanxue-22 force-pushed the DX-1301-Fix-Lost-Request-Data-Using-Middleware branch 4 times, most recently from 3e3f0c1 to f9830c9 Compare October 17, 2025 22:34
@yanxue-22 yanxue-22 marked this pull request as ready for review October 17, 2025 22:37
@yanxue-22 yanxue-22 requested a review from a team as a code owner October 17, 2025 22:37
@yanxue-22 yanxue-22 force-pushed the DX-1301-Fix-Lost-Request-Data-Using-Middleware branch 2 times, most recently from d614502 to eda9765 Compare October 20, 2025 01:54
@yanxue-22 yanxue-22 force-pushed the DX-1301-Fix-Lost-Request-Data-Using-Middleware branch from eda9765 to ee2e4c1 Compare October 20, 2025 03:01
@yanxue-22 yanxue-22 force-pushed the DX-1301-Fix-Lost-Request-Data-Using-Middleware branch from e5b9091 to 220b32d Compare October 20, 2025 18:13
@ericcrosson-bitgo ericcrosson-bitgo self-requested a review October 20, 2025 20:32
@yanxue-22
Copy link
Contributor Author

yanxue-22 commented Oct 20, 2025

Closing this ticket because it is working as designed. The issue occurs when defining handlers without the routeHandler() wrapper. This legacy path uses:

export async function runMiddlewareChainIgnoringResults<

This explicitly ignores middleware results and only returns the original decoded request input.

@yanxue-22 yanxue-22 closed this Oct 20, 2025
@yanxue-22 yanxue-22 deleted the DX-1301-Fix-Lost-Request-Data-Using-Middleware branch November 4, 2025 01:04
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 this pull request may close these issues.

Request data lost using "normal express middleware"

2 participants