GenHTTP icon indicating copy to clipboard operation
GenHTTP copied to clipboard

Add modern web socket implementation

Open MDA2AV opened this issue 1 month ago • 9 comments

Add new websocket RFC-6455 compliant module.

MDA2AV avatar Nov 28 '25 10:11 MDA2AV

@MDA2AV so the web socket client of the integration test hangs indefinitely on line 33, and most of the time only on .NET 10. Need to check what the issue is here ...

In the meantime I extended the connection to be able to have access to the original request. This prevents us from caching the content but I think this is still desired by the users.

Kaliumhexacyanoferrat avatar Dec 02 '25 07:12 Kaliumhexacyanoferrat

@MDA2AV so the web socket client of the integration test hangs indefinitely on line 33, and most of the time only on .NET 10. Need to check what the issue is here ...

In the meantime I extended the connection to be able to have access to the original request. This prevents us from caching the content but I think this is still desired by the users.

Hmm so the issue seems to be the ClientWebSocket class? Maybe we could try using the WebsocketClient instead as in the legacy integration tests but this is wierd

MDA2AV avatar Dec 02 '25 08:12 MDA2AV

Well, there is certainly a reason why this happens, we just do not know yet, why. I fear the only solution is to put console log statements everywhere 🙄

At least the parser does not emit a ErrorFrame.

Kaliumhexacyanoferrat avatar Dec 02 '25 08:12 Kaliumhexacyanoferrat

I'll take a look at it, could be a .NET 10 issue with ClientWebsocket, interestingly I think when I run tests on my machine they were passing, I'll check again and if there are people having same issue

MDA2AV avatar Dec 02 '25 08:12 MDA2AV

Just had a build where .NET 8 fails. But most of the time it is .NET 10 - could be a timing thing because .NET 10 is executed last.

Kaliumhexacyanoferrat avatar Dec 02 '25 08:12 Kaliumhexacyanoferrat

I was thinking whether the PoolBufferedStream which is underlying as target does not flush in all cases, but actually the code looks fine to me. Also this would probably have noticed elsewhere already (in SSE or the old Websocket implementation).

Kaliumhexacyanoferrat avatar Dec 02 '25 09:12 Kaliumhexacyanoferrat

I feel like it's something simpler, i ran tests 100 times and failed a few times on the imperative integration only, the reactive integration tests passed always, did you get same?

MDA2AV avatar Dec 02 '25 09:12 MDA2AV

Did not manage to reproduce the error locally on Windows (but also not on the Windows workflow runners at it seems).

Kaliumhexacyanoferrat avatar Dec 02 '25 09:12 Kaliumhexacyanoferrat

Missing:

  • Unit tests for new Decode method
  • Integration tests for fragmented and pipelined frames

MDA2AV avatar Dec 02 '25 19:12 MDA2AV

Follow-up tickets

#775 #776 #777

Kaliumhexacyanoferrat avatar Dec 09 '25 16:12 Kaliumhexacyanoferrat