Skip to content

Fix support for node 11.1 and above#13

Merged
indutny merged 2 commits into
spdy-http2:masterfrom
Cellule:node-11
Mar 25, 2020
Merged

Fix support for node 11.1 and above#13
indutny merged 2 commits into
spdy-http2:masterfrom
Cellule:node-11

Conversation

@Cellule

@Cellule Cellule commented Dec 10, 2019

Copy link
Copy Markdown
Contributor

Fixes #6

Took a stab at trying to fix onread support for node 11.1 and above.
Looked at https://github.com/nodejs/node/blob/v11.1.0/lib/internal/stream_base_commons.js#L111 to figure out how to pass nread
Confirmed on master https://github.com/nodejs/node/blob/master/lib/internal/stream_base_commons.js#L163 that the implement hasn't changed since then


This change is Reviewable

@mathieumg

mathieumg commented Dec 10, 2019

Copy link
Copy Markdown

Related: #7 and #12

@matthewmeyer

Copy link
Copy Markdown

Yay! can we get a release?

@ovasylenko

Copy link
Copy Markdown

Yeap. Looking forward to release

@leonardocartaxo

Copy link
Copy Markdown

Has this fix been released? I'm just ran into this error.

not on the last version 4.0.1

@leonardocartaxo

Copy link
Copy Markdown

We need this fix asp, please. We are struggling with this error for months.

@abhinavsingi

Copy link
Copy Markdown

When will this be released?
As per my understanding w/o this spdy is not supported on node@12 ?

@juliendoyon

Copy link
Copy Markdown

Is there any date scheduled to release this fix?

@aaclayton

Copy link
Copy Markdown

Please merge!

@pgowdy1

pgowdy1 commented Mar 2, 2020

Copy link
Copy Markdown

I just want to echo what aaclayton said. Please, please merge this fix.

@HelmerBarcos

HelmerBarcos commented Mar 11, 2020

Copy link
Copy Markdown

@daviddias @jacobheun What about this PR for adding http2 support to node 12 LTS ??

@Cellule

Cellule commented Mar 17, 2020

Copy link
Copy Markdown
Contributor Author

Since this doesn't seem to be getting merged anytime soon, I thought I'd share my patch
https://gist.github.com/Cellule/91c77e67b6edc640934a492fcba67239
You can install that patch using patch-package

@indutny indutny left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@indutny indutny merged commit dd2de6b into spdy-http2:master Mar 25, 2020
@indutny

indutny commented Mar 25, 2020

Copy link
Copy Markdown
Collaborator

Published as 2.0.1

@aphix

aphix commented Mar 26, 2020

Copy link
Copy Markdown

Published as 2.0.1

Many Thanks, @indutny!

@adityashukla74

Copy link
Copy Markdown

It still fails on node 12 in mid-Sept 2020.

@leonardocartaxo

Copy link
Copy Markdown

Works for me in node 14. Update to the last version and it should be working fine

@aphix

aphix commented Sep 18, 2020

Copy link
Copy Markdown

@adityashukla74 Make sure you've cleared node_modules and reinstalled, double check your package lock and ensure that handle-thing is >= 2.0.1 and it should work, however ensure you test it locally (by spamming refresh or something to create/kill some reqs before they can complete) before going to prod with newer versions of v12 due to this (unrelated) issue:

spdy-http2/node-spdy#369

Hopefully this patch to handle-thing will be merged soon to resolve it:

#16

@adityashukla74

adityashukla74 commented Sep 21, 2020

Copy link
Copy Markdown

@aphix I tried with Node v12.13.0 LTS.
Steps followed-
node_modules removed, npm cache clean, removed package-lock.json, mentioned "spdy": "^4.0.2", and "handle-thing": "^2.0.1", in package.json, reinstalled the packages and restarted build

webpack: Compiled successfully.
(node:59063) [DEP0066] DeprecationWarning: OutgoingMessage.prototype._headers is deprecated
(Use node --trace-deprecation ... to show where the warning was created)
internal/buffer.js:945
class FastBuffer extends Uint8Array {}
^

RangeError: Invalid typed array length: -4095
at new Uint8Array ()
at new FastBuffer (internal/buffer.js:945:1)
at Handle.onStreamRead [as onread] (internal/stream_base_commons.js:185:19)
at Stream. (node_modules/webpack-dev-server/node_modules/handle-thing/lib/handle.js:120:12)
at Stream.emit (events.js:327:22)
at endReadableNT (node_modules/webpack-dev-server/node_modules/readable-stream/lib/_stream_readable.js:1010:12)
at processTicksAndRejections (internal/process/task_queues.js:84:21)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! web@2.65.1-0 start: ./scripts/server.sh "--https"
npm ERR! Exit status 1
npm ERR!
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
Stopping proxy

npm ERR! A complete log of this run can be found in:
npm ERR! /Users/adityashukla/.npm/_logs/2020-09-21T06_30_49_478Z-debug.log

@aphix

aphix commented Sep 23, 2020

Copy link
Copy Markdown

That's a different issue @adityashukla74 -- also I've looked further and that solution isn't viable in either case.

That sounds like this package using something not available to your webpack build and far outside the scope of this package.

I think you might be using a wrong tool for a job somewhere, good luck.

@aphix

aphix commented Sep 23, 2020

Copy link
Copy Markdown

that does look like the old error though... what are the outputs of node -v && npm -v , @adityashukla74?

patrickkokou added a commit to Voxer/node-spdy that referenced this pull request May 16, 2026
…on Node 16+

Two follow-ups to the earlier deceiver fix; both surfaced when bringing
up gcp2-dev03-00 on Node 24.

1. handle-thing@^1.2.5 calls socket._handle.onread with the old
   (nread, buffer) signature. On Node 11.1+, the handle's onread is
   actually node:internal/stream_base_commons.onStreamRead(arrayBuffer),
   which reads nread from streamBaseState[kReadBytesOrError]. Passing
   UV_EOF (-4095) as the arrayBuffer arg made onStreamRead try
   `new FastBuffer(-4095, ...)` and crash:

       RangeError: Invalid typed array length: -4095
           at new FastBuffer (node:internal/buffer:956:1)
           at Handle.onStreamRead [as onread]
              (node:internal/stream_base_commons:188:19)
           at Stream.<anonymous> (.../handle-thing/lib/handle.js:120:12)

   handle-thing@2.0.1 (spdy-http2/handle-thing#13) translates the
   call: it sets streamBaseState[kReadBytesOrError] first and then
   invokes onread with just the buffer. It also adds .writev which
   modern net.Socket needs. Bump the dependency.

2. The previous deceiver fallback used `require('http').METHODS` for
   the methods list. http.METHODS is alphabetically sorted, but the
   parser's internal index is the llhttp/http_parser order
   (DELETE, GET, HEAD, POST, ...). Sending the alphabetical index made
   Node interpret POST as ACL etc. Use `_http_common.methods`
   (or `node:_http_common.methods`), which is the real parser-indexed
   list.

Smoke-tested end-to-end on Node v24.11.1: spdy.createServer with TLS
+ h2 alpn, hit with the built-in http2 client, POST /, 11-byte body,
server sees method=POST, body bytes flow through handle-thing's data
path, EOF path completes without the -4095 crash, client receives
200 OK with the response body.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

node 11.1.0 onStreamRead params have changed