Skip to content

Conversation

@tegefaulkes
Copy link
Contributor

@tegefaulkes tegefaulkes commented Feb 9, 2024

Description

This is take two of the previous PR at #86

This PR attempts to fix the problem with socket send errors not being handled properly when they're specific to the connection triggering the send. It also handles basic network dropouts.

The previous PR had some issues with platform specific behaviour with the send errors. The 3 different platforms gave 3 different error codes for invalid IP based on how the socket was bound.

Issues Fixed

Tasks

  • 1. Send errors specific to the connection triggering send should bubble back to that connection to be handled properly.
  • 2. Starting a connection should fail if the target address is invalid. In this case due to external addresses being invalid for loop back bound sockets.
  • 3. Temp network failure does not cause a crash but connections eventually time out.
  • 4. Fix problems with the CI failures

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

@tegefaulkes tegefaulkes self-assigned this Feb 9, 2024
@ghost
Copy link

ghost commented Feb 9, 2024

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

This commit addresses the send failures by taking any send specific errors and passing them back to the connection to be handled.
Any errors such as bad arguments results in the connection throwing the problem proactivity.
Any network failures are generally ignored and the Connections are left to time out. This allows for the network to drop for a short amount of time without failure of the connection and streams.

[ci skip]
There were a few things here. For starters some of the new tests had some dumb config. A mistake on my part.

The other part is that the `send` throws different errors depending on platform and context. I could only reasonably track it down based on testing in the CI... There are 3 main errors we're sending back to the connection to be dealt with.
@tegefaulkes tegefaulkes force-pushed the feature-socket-errors branch from 8876106 to 6b62491 Compare February 9, 2024 06:22
@tegefaulkes
Copy link
Contributor Author

There was mention of some odd behaviour when switching wifi networks which needs to be looked into. But I'll make a new issue for that.

@tegefaulkes tegefaulkes merged commit eaa517d into staging Feb 12, 2024
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.

js-quic hard crashes when bound to loopback and creating connection to an external address.

2 participants