Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: issues with packet broadcast error reporting #2549

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

JFreegman
Copy link
Member

@JFreegman JFreegman commented Jan 11, 2024

commit 5b9c420 introduced some undesirable behaviour with packet send functions returning error when they shouldn't. We now only return an error if the packet fails to be added to the send queue or cannot be wrapped/encrypted. We no longer error if we fail to send the packet over the wire, because toxcore will keep trying to re-send the packet until the connection times out.

Additionally, we now make sure that our packet broadcast functions aren't returning an error when failing to send packets to peers that we have not successfully handshaked with yet, since this is expected behaviour.


This change is Reviewable

@JFreegman JFreegman added the bug Bug fix for the user, not a fix to a build script label Jan 11, 2024
@JFreegman JFreegman added this to the v0.2.19 milestone Jan 11, 2024
@JFreegman JFreegman force-pushed the group_send_error_fix branch 4 times, most recently from f7ba009 to 4ea38ec Compare January 11, 2024 18:45
Copy link

codecov bot commented Jan 11, 2024

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (6b6718e) 69.02% compared to head (072e3be) 69.02%.

Files Patch % Lines
toxcore/group_connection.c 27.27% 8 Missing ⚠️
toxcore/group_chats.c 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2549      +/-   ##
==========================================
- Coverage   69.02%   69.02%   -0.01%     
==========================================
  Files          96       96              
  Lines       28049    28054       +5     
==========================================
+ Hits        19362    19364       +2     
- Misses       8687     8690       +3     

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

@JFreegman JFreegman force-pushed the group_send_error_fix branch from e09a230 to 01608a1 Compare January 11, 2024 19:14
commit 5b9c420 introduced some undesirable behaviour with packet send
functions returning error when they shouldn't. We now only return an
error if the packet fails to be added to the send queue or cannot
be wrapped/encrypted. We no longer error if we fail to send the packet
over the wire, because toxcore will keep trying to re-send the packet
until the connection times out.

Additionally, we now make sure that our packet broadcast functions
aren't returning an error when failing to send packets to peers
that we have not successfully handshaked with yet, since this is
expected behaviour.
@JFreegman JFreegman force-pushed the group_send_error_fix branch from 01608a1 to 072e3be Compare January 11, 2024 19:55
@JFreegman JFreegman merged commit 072e3be into TokTok:master Jan 11, 2024
56 of 57 checks passed
@JFreegman JFreegman deleted the group_send_error_fix branch January 11, 2024 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug fix for the user, not a fix to a build script
Development

Successfully merging this pull request may close these issues.

2 participants