Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mount for Node.js >= 15.1.0 #66

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Commits on Dec 12, 2023

  1. 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] 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"
    kyptin committed Dec 12, 2023
    Configuration menu
    Copy the full SHA
    b4f06c4 View commit details
    Browse the repository at this point in the history