-
Notifications
You must be signed in to change notification settings - Fork 80
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
feat: Supports callbacks when reading a message fails #331
base: main
Are you sure you want to change the base?
Conversation
|
cbc48e1
to
2ed37ef
Compare
Where is the new callback used? |
Sorry, missed submitting a file change. |
This conflicts with other changes to callbacks. Please recreate if still relevant. |
server/serverimpl.go
Outdated
var ( | ||
errAlreadyStarted = errors.New("already started") | ||
) | ||
var errAlreadyStarted = errors.New("already started") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't make irrelevant changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
server/types/callbacks.go
Outdated
@@ -62,6 +62,9 @@ type ConnectionCallbacks struct { | |||
|
|||
// OnConnectionClose is called when the OpAMP connection is closed. | |||
OnConnectionClose func(conn Connection) | |||
|
|||
// OnConnectionError is called when an error occurs while reading or serializing a message. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// OnConnectionError is called when an error occurs while reading or serializing a message. | |
// OnReadMessageError is called when an error occurs while reading or deserializing a message. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
server/serverimpl_test.go
Outdated
defer conn.Close() | ||
|
||
// Send a message to the Server. | ||
err := conn.WriteMessage(websocket.TextMessage, []byte("")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include some payload to verify it gets delivered to the callback ,e.g.:
err := conn.WriteMessage(websocket.TextMessage, []byte("")) | |
err := conn.WriteMessage(websocket.TextMessage, []byte("abc")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
server/serverimpl_test.go
Outdated
eventually(t, func() bool { return rcvMsg.Load() != nil }) | ||
errInfo := rcvMsg.Load().(ErrorInfo) | ||
assert.EqualValues(t, websocket.TextMessage, errInfo.mt) | ||
assert.EqualValues(t, []byte(""), errInfo.msgByte) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert.EqualValues(t, []byte(""), errInfo.msgByte) | |
assert.EqualValues(t, []byte("abc"), errInfo.msgByte) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #331 +/- ##
==========================================
+ Coverage 78.29% 78.46% +0.17%
==========================================
Files 25 25
Lines 2386 2401 +15
==========================================
+ Hits 1868 1884 +16
Misses 410 410
+ Partials 108 107 -1 ☔ View full report in Codecov by Sentry. |
1b1915e
to
8335e42
Compare
@tttoad please fix the build. |
see: #330