Open
Conversation
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"
Author
|
It seems this patch breaks |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I tried the basic example in the README. It failed with a 500 Internal Server Error and the following JSON body:
The output from the server process includes a stack trace. Sanitized a bit, it is:
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
nvmto 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.assignon 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 thinkconverge) do this implicitly, as does funky'sassocWith. 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:
In which I added an index.js file with these contents:
And finally (with
fdandentrandhttpieinstalled), I started a file-watching auto-test process: