network: add OutgoingMessage disconnectReason field #5147
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5147 +/- ##
==========================================
- Coverage 53.56% 53.54% -0.03%
==========================================
Files 433 433
Lines 54736 54736
==========================================
- Hits 29321 29310 -11
- Misses 23136 23141 +5
- Partials 2279 2285 +6
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
cce
force-pushed
the
OutgoingMessage-disconnectReason
branch
from
ebd951b to
a2503f6
Compare
| return OutgoingMessage{Broadcast, msg.Tag, msg.Data, nil} | ||
| return OutgoingMessage{Action: Broadcast, Tag: msg.Tag, Payload: msg.Data, Topics: nil} |
There was a problem hiding this comment.
was this a required change? just wondering because I don't see a difference besides adding the attribute names.
There was a problem hiding this comment.
yes, adding an extra field made struct initializations not work here because all the fields have to be specified if you aren't providing the field names.
| reason := disconnectBadData | ||
| if outmsg.reason != disconnectReasonNone { | ||
| reason = outmsg.reason | ||
| } | ||
| go wn.disconnectThread(msg.Sender, reason) |
There was a problem hiding this comment.
clever! I was thinking about the work required to include the badData to all disconnect messages, but this is way cleaner.
There was a problem hiding this comment.
yes, it is the current default and is already being set in this line, so I kept it set as default here in the line I replaced it with
Summary
Following up on feedback from #4695, this adds an unexported disconnectReason field to the OutgoingMessage field for use with tracking internal network handlers' reasons for disconnecting.
Test Plan
Extended TestWebsocketDisconnection to use this new feature and assert the correct reason was reported.