Skip to content

Commit

Permalink
Fix mount for Node.js >= 15.1.0
Browse files Browse the repository at this point in the history
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] nodejs/node#35281
[2] nodejs/node#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] nodejs/node#36550 (comment)

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"
  • Loading branch information
kyptin committed Dec 12, 2023
1 parent ef30068 commit b4f06c4
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 32 deletions.
11 changes: 6 additions & 5 deletions lib/mount.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
38 changes: 18 additions & 20 deletions lib/parseUrl.js
Original file line number Diff line number Diff line change
@@ -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
}
11 changes: 4 additions & 7 deletions lib/passThruBody.js
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit b4f06c4

Please sign in to comment.