From b4f06c4f575c46b1e62e4dd6cbb298cdc3751552 Mon Sep 17 00:00:00 2001 From: Jeff Terrell Date: Mon, 11 Dec 2023 19:49:32 -0500 Subject: [PATCH] Fix `mount` for Node.js >= 15.1.0 I tried the basic example in the README. It failed with a 500 Internal Server Error and the following JSON body: { "message": "Cannot read properties of undefined (reading 'x-forwarded-proto')", "name": "TypeError" } The output from the server process includes a stack trace. Sanitized a bit, it is: TypeError: Cannot read properties of undefined (reading 'x-forwarded-proto') at protocol (.../node_modules/paperplane/lib/parseUrl.js:18:14) at .../node_modules/ramda/src/converge.js:47:17 at _map (.../node_modules/ramda/src/internal/_map.js:6:19) at .../node_modules/ramda/src/converge.js:46:33 at f1 (.../node_modules/ramda/src/internal/_curry1.js:18:17) at .../node_modules/ramda/src/uncurryN.js:34:21 at .../node_modules/ramda/src/internal/_curryN.js:37:27 at .../node_modules/ramda/src/internal/_arity.js:10:19 at .../node_modules/ramda/src/internal/_pipe.js:3:14 at .../node_modules/ramda/src/internal/_arity.js:10:19 Yet the docker-based demo app works. That uses Node.js v12.22.12. When I tried that Node version on the basic example, it also worked. So, I used `nvm` to do a binary search on versions. I identified that Node.js v15.1.0 is the first failing version, and every version I tried that was newer (up to v21.2.0) also failed. Scouring the Node.js change log revealed that the http module of v15.1.0 started calculating req.headers lazily. There's a full discussion in nodejs/node#35281 [1]. Note the referenced issue that identifies the bug [2]. [1] https://github.com/nodejs/node/pull/35281 [2] https://github.com/nodejs/node/pull/36550 The workaround identified in this comment [3] is not to use the spread operator or `Object.assign` on the request object. "These objects are essentially uncloneable." [3] https://github.com/nodejs/node/issues/36550#issuecomment-747983715 This is surprisingly tricky to do right. Various Ramda functions like `merge` (and I think `converge`) do this implicitly, as does funky's `assocWith`. So to work around this limitation, while preserving all behavior, I had to resort to (gasp) mutating procedures instead of pure functions. Not ideal, I know. I'm open to better ideas. So, maybe this isn't the best solution, but it least it gets the example working again on modern Node versions. If it never gets merged, at least it will be findable via the repo on GitHub. For reference, to test this, I used a fresh NPM project with only the paperplane dependency: mkdir paperplane cd paperplane npm init -y npm i -S paperplane In which I added an index.js file with these contents: const { compose } = require('ramda') const http = require('http') const { json, logger, methods, mount, routes } = require('paperplane') const cookies = req => ({ cookies: req.cookies || 'none?' }) const hello = req => ({ message: `Hello ${req.params.name}!` }) const app = routes({ '/' : methods({ GET: _ => ({ body: 'Hello anon.' }) }), '/cookies' : methods({ GET: compose(json, cookies) }), '/hello/:name': methods({ GET: compose(json, hello) }) }) http.createServer(mount({ app })).listen(3000, logger) And finally (with `fd` and `entr` and `httpie` installed), I started a file-watching auto-test process: fd -e js | entr -rcc bash -c "node index.js & http -v get http://localhost:3000/cookies Cookie:foo=bar" --- lib/mount.js | 11 ++++++----- lib/parseUrl.js | 38 ++++++++++++++++++-------------------- lib/passThruBody.js | 11 ++++------- 3 files changed, 28 insertions(+), 32 deletions(-) diff --git a/lib/mount.js b/lib/mount.js index b335fc0..287aa53 100644 --- a/lib/mount.js +++ b/lib/mount.js @@ -12,11 +12,12 @@ const { wrap } = require('./wrap') const { writeHTTP } = require('./writeHTTP') const { writeLambda } = require('./writeLambda') -const keepOriginal = curry((response, request) => merge(request, { - original: request, // legacy - request, - response -})) +const keepOriginal = res => req => { + req.original = req + req.request = req + req.response = res + return req +} exports.mount = (opts={}) => { const { diff --git a/lib/parseUrl.js b/lib/parseUrl.js index ef522d6..2cde11a 100644 --- a/lib/parseUrl.js +++ b/lib/parseUrl.js @@ -1,25 +1,23 @@ -const { assocWith } = require('@articulate/funky') const qs = require('qs') const url = require('url') -const { - converge, evolve, identity, merge, pick, pipe, prop -} = require('ramda') +const protocol = ({ headers, socket }) => + headers['x-forwarded-proto'] || + socket.encrypted ? 'https' : 'http' -const pathParts = - pipe( - prop('url'), - url.parse, - evolve({ query: qs.parse }), - pick(['pathname', 'query']) - ) +const pathParts = req => { + const { pathname, query } = url.parse(req.url) + return { + pathname, + query: qs.parse(query), + protocol: protocol(req), + } +} -const protocol = req => - req.headers['x-forwarded-proto'] || - req.socket.encrypted ? 'https' : 'http' - -exports.parseUrl = - pipe( - converge(merge, [ identity, pathParts ]), - assocWith('protocol', protocol) - ) +exports.parseUrl = req => { + const { pathname, query, protocol } = pathParts(req) + req.pathname = pathname + req.query = query + req.protocol = protocol + return req +} diff --git a/lib/passThruBody.js b/lib/passThruBody.js index 6a9b534..9c937a9 100644 --- a/lib/passThruBody.js +++ b/lib/passThruBody.js @@ -1,11 +1,8 @@ -const { assocWith } = require('@articulate/funky') const { PassThrough } = require('stream') -const passThru = ({ original }) => { +exports.passThruBody = req => { const body = new PassThrough() - original.pipe(body) - return body + req.original.pipe(body) + req.body = body + return req } - -exports.passThruBody = - assocWith('body', passThru)