Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.50%. Comparing base (ef25209) to head (230b784).
⚠️ Report is 979 commits behind head on main.

@@            Coverage Diff             @@
##             main    #2999      +/-   ##
==========================================
- Coverage   93.50%   93.50%   -0.01%     
==========================================
  Files          89       89              
  Lines       24223    24220       -3     
==========================================
- Hits        22649    22646       -3     
  Misses       1574     1574              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ACtually doesn't this just move the issue to when websocket is used? Should we maybe not pass fatal? Does the spec require that? If it does, then maybe we should disable websocket for builds without ICU?

I don't particularly care how this moves forward but the revert is needed to unblock a Node.js version bump.

It actually revealed that there is an issue with no-icu cases.

Which, now that we know about it, we can take all the time in the world to contemplate a fix for. In the meantime, node's bundled fetch is lagging behind on fixes which is why this revert is proposed.

I would atleast expect that you open an issue here, so that it keeps tracked.

I would atleast expect that you open an issue here, so that it keeps tracked.

Noted.

#3002

@mcollina can you do a release? Node nightly is currently lagging on 6.6.2

6.10.2 published

Does the spec require that?

yes, we're meant to close the connection if we receive invalid utf-8

If it does, then maybe we should disable websocket for builds without ICU?

It actually revealed that there is an issue with no-icu cases.

It's already essentially disabled:

• Open a WebSocket connection (on a build without icu)
• The connection closes if it receives a message.
• The close frame has no reason if we receive a close frame.

We might be able to support builds without icu by using Buffer.isUTF8, but it should only be used when we can't use TextDecoder for perf. I'm not really sure if it's available in builds without ICU and I don't have the time to test myself.