Web3.js: XMLHttpRequest errors are not handled correctly

[liamaharon]

In the event of an error occurring during a XMLHttpRequest, onreadystatechange is called when

• request.readyState is 4
• request.responseText is ""
• request.status is 0

The following code that handles http requests does not handle this case, and can return a misleading error to the user when it occurs

https://github.com/ethereum/web3.js/blob/dee72e00883c5a23a47b6d3c1d27304ab1d4922e/packages/web3-providers-http/src/index.js#L87-L125

Let's pretend a machine has exhausted all of it's ephemeral ports right before web3.js tries to send a request. Here's what happens

1. HttpProvider.prototype.send is called
2. The request is crafted
3. onreadystatechange and ontimeout handlers are assigned
4. request.send is called with the payload
5. The request fails before it is even sent to the destination due to there being no ephemeral ports available
6. request.readyState is set to 4, request.responseText is set to "", request.status is set to 0 and the onreadystatechange handler is called
7. web3.js tries to parse "" and throws an error that looks like this

  <rejected> Error: Invalid JSON RPC response: ""
      at Object.InvalidResponse (/home/liam/spells/web3.js/packages/web3-core-helpers/src/errors.js:42:16)
      at XMLHttpRequest.request.onreadystatechange (/home/liam/spells/web3.js/packages/web3-providers-http/src/index.js:108:32)
      at XMLHttpRequestEventTarget.dispatchEvent (/home/liam/spells/web3.js/packages/web3-providers-http/node_modules/xhr2-cookies/dist/xml-http-request-event-target.js:34:22)
      at XMLHttpRequest._setReadyState (/home/liam/spells/web3.js/packages/web3-providers-http/node_modules/xhr2-cookies/dist/xml-http-request.js:208:14)
      at XMLHttpRequest._onHttpRequestError (/home/liam/spells/web3.js/packages/web3-providers-http/node_modules/xhr2-cookies/dist/xml-http-request.js:349:14)
      at ClientRequest.<anonymous> (/home/liam/spells/web3.js/packages/web3-providers-http/node_modules/xhr2-cookies/dist/xml-http-request.js:252:61)
      at ClientRequest.emit (events.js:210:5)
      at ClientRequest.EventEmitter.emit (domain.js:475:20)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at Socket.emit (events.js:210:5)
      at Socket.EventEmitter.emit (domain.js:475:20)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

this error is misleading because it indicates that the node successfully received the request and actually returned an empty string in response, when in reality the request never even made it to the node and an empty string is just the default value set on request in an error scenario.

I believe this may be the cause of some other issues people have opened in the past like https://github.com/ethereum/web3.js/issues/3370 and https://github.com/ethereum/web3.js/issues/1919

I think it makes sense to at least add an onerror handler to the request, and in the case that a request fails the error thrown indicates that instead of a misleading InvalidResponse error.

I'm working on a PR for this.

[cgewecke]

@liamaharon Thanks for such a great description and analysis of this problem.

[wbt]

As an FYI: With the error lacking any particular code to check on, we and probably others are checking for a string match and when the error message changes to something more informative, that change will probably break our ability to handle it robustly (mainly by waiting a bit and trying again). We will need to coordinate code changes around the time of upgrade to adjust for that breaking change. Therefore, I would definitely recommend against rolling out a fix for this on e.g. the patch version number.

[github-actions[bot]]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment, otherwise this issue will be closed in 7 days

[wbt]

I don't think this should be marked as stale.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

[wbt]

I still don't think this should be marked as stale.

[uluhonolulu]

I wish I could do something to help with this, I keep seeing this error and have to write dirty workaround code to handle it every time.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

[liamaharon]

@uluhonolulu I have a PR in progress that addresses this, TODO is to get error codes thrown instead of strings. I don't have time to get around to finishing it right now, if you're still willing & able to help would be great if you could assist finishing it off https://github.com/ethereum/web3.js/pull/3426

[uluhonolulu]

@liamaharon I'll be happy to work on it. Do you think I'll be able to continue working on your PR (permissions?) or create a new one? (Sorry if it's a dumb question)

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

[wbt]

@liamaharon Did you wind up being able to make any progress? It looks like you got an answer to that question on the PR!