Fix RPC Server Error: TypeError: Cannot read property 'readable' of undefined at IncomingMessage._read (_http_incoming.js:120:19)

[jeremygiberson]

Summary

When attempting to work through the reach-sh guides I found that the RPC Server reach rpc-server would not handle any requests. Any request would crash rpc server with the following output:

$ sh serve.sh 
Warning! Using development RPC key: REACH_RPC_KEY=opensesame.
Warning! The current TLS certificate is only suitable for development purposes.
Verifying knowledge assertions
Verifying for generic connector
  Verifying when ALL participants are honest
  Verifying when NO participants are honest
  Verifying when ONLY "Alice" is honest
  Verifying when ONLY "Bob" is honest
Checked 17 theorems; No failures!

> @reach-sh/rpc-server@ app /app
> node --experimental-modules --unhandled-rejections=strict index.mjs

2022-01-01T17:41:08.488Z DEBUG: address_ethToCfx call 0x0000000000000000000000000000000000000000
2022-01-01T17:41:08.563Z DEBUG: I am alive
_http_incoming.js:120
  if (this.socket.readable)
                  ^

TypeError: Cannot read property 'readable' of undefined
    at IncomingMessage._read (_http_incoming.js:120:19)
    at IncomingMessage.Readable.read (internal/streams/readable.js:481:10)
    at resume_ (internal/streams/readable.js:977:12)
    at processTicksAndRejections (internal/process/task_queues.js:82:21)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @reach-sh/rpc-server@ app: `node --experimental-modules --unhandled-rejections=strict index.mjs`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the @reach-sh/rpc-server@ app script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2022-01-01T17_41_10_606Z-debug.log

In the above crash, I attempted to perform the following request:

curl -ik -X POST -H 'X-API-Key: opensesame' -H 'Content-Type: application/json; charset=utf-8' https://127.0.0.1:3000/stdlib/parseCurrency -d '[4]'

I did some investigation and found this issue for express library: expressjs/express#3388

Apparently, the current version of express (v4) is not compatible with http2. Suggestions are to upgrade to express v5 or use spdy for http2. Since v5 is still in alpha stage, decided to use the more stable spdy library in place of http2.

DESIGN

There are no fundamental design changes or impact from this change.

TESTING

I used the following test scenario (On mac Mojave 10.14.6):

1. Using project complete the reach-tut for rock paper scissors
2. start rpc-server sh ./serve.sh

$ REACH_DEBUG=* REACH_RPC_PORT=3000 REACH_CONNECTOR_MODE=ETH ~/reach rpc-server
Warning! Using development RPC key: REACH_RPC_KEY=opensesame.
Warning! The current TLS certificate is only suitable for development purposes.
Verifying knowledge assertions
Verifying for generic connector
  Verifying when ALL participants are honest
  Verifying when NO participants are honest
  Verifying when ONLY "Alice" is honest
  Verifying when ONLY "Bob" is honest
Checked 17 theorems; No failures!

> @reach-sh/rpc-server@ app /app
> node --experimental-modules --unhandled-rejections=strict index.mjs

2022-01-01T18:49:49.943Z DEBUG: address_ethToCfx call 0x0000000000000000000000000000000000000000
2022-01-01T18:49:50.021Z DEBUG: I am alive (with spdy)

3. Issue a curl request to to rpc server:

$ curl -ik -X POST -H 'X-API-Key: opensesame' -H 'Content-Type: application/json; charset=utf-8' https://127.0.0.1:3000/stdlib/parseCurrency -d '[4]'
HTTP/2 200 
content-type: application/json; charset=utf-8
content-length: 47
etag: W/"2f-Vu0VXEJaX6DEbqJzyQltFH8v+4Q"

{"type":"BigNumber","hex":"0x3782dace9d900000"}

DOCS

This is a trivial change to the internals of rpc-server operation that doesn't warrant any documentation updates.

[jeapostrophe]

Awesome, this looks great. I'm going to ask @mattaudesse to evaluate it on Monday and merge it :)

[mattaudesse]

Hey @jeremygiberson, thanks a bunch for looking into this and offering a fix - much appreciated!

The TL;DR answer to this wart is to just append --http1.1 when using curl, but I'll try to motivate why we made this tradeoff below (and why we've stuck with it despite the temporary clunkiness):

• SPDY is a deprecated protocol: https://en.wikipedia.org/wiki/SPDY

  SPDY became the basis for HTTP/2 specification. However, HTTP/2 diverged from SPDY and eventually HTTP/2 subsumed all usecases of SPDY.[7] After HTTP/2 was ratified as a standard, major implementers, including Google, Mozilla, and Apple, deprecated SPDY in favor of HTTP/2. As of 2021, no modern browser supports SPDY.

• What we really want is for express to work out of the box with HTTP/2 and ALPN: https://en.wikipedia.org/wiki/Application-Layer_Protocol_Negotiation.
• I considered making everything just use HTTP/1.1 at first, but ultimately decided that while building the various RPC clients it's ideal to target HTTP/2 optimistically for now, leveraging the transparent HTTP/1.1-fallback behavior that's baked into most libraries until there's a stable express release that truly supports HTTP/2. That makes for less work, less code/branching complexity, and seamless transition when we can flip the express switch later.
• As a tool for power users curl is arguably doing the Right Thing here by failing instead of implementing a transparent fallback like libraries that are more concerned with convenience and just serving a request. As mentioned above, you could use --http1.1 or, alternatively, --http2 --no-alpn -v (but the -v will show you still receive an HTTP/1.1 response for now).
• The node-spdy library has an open issue stating it's incompatible with node >= v15 :-)

We'd much prefer users not to have to care about any of this, so I'm very tempted to just click merge, but for now I think it makes the most sense to keep the server as-is and try to provide rationale when folks like you rightfully call it out. Thanks again for investing the time, though!