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

added natsConn_(get|set)(Error|Closed)Callback #647

Closed
wants to merge 4 commits into from

Conversation

levb
Copy link
Collaborator

@levb levb commented Apr 6, 2023

To use in the microservices implementation, to wrap connection handlers on an active connection.

(still need to add a test for setClosedCallback)

@levb levb requested a review from kozlovic April 6, 2023 20:42
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are failing (does not seem to be flapping since the same tests fail in various matrix runs). It could be because of the extra break that I pointed out, but not sure.

src/nats.c Outdated Show resolved Hide resolved
test/test.c Outdated Show resolved Hide resolved
src/nats.c Outdated Show resolved Hide resolved
@levb
Copy link
Collaborator Author

levb commented Apr 6, 2023

Oops, @kozlovic sorry I tagged you too early; I pushed to run the tests, still finishing this up. Thanks for jumping onto it!

@levb levb requested a review from kozlovic April 7, 2023 17:06
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just a note, since recently, it is assumed that the async error callback will always be set (if not set by the user, we use a default handler that prints to stderr, see natsConn_defaultErrHandler). I see that your new setter does not allow NULL, so there is no risk at the moment that you would remove the error handler. If you do in the future, check invocations of natsAsyncCb_PostErrHandler that again assume that a callback is always set.

@levb levb mentioned this pull request Apr 26, 2023
@levb
Copy link
Collaborator Author

levb commented Apr 26, 2023

#640 has an improved implementation, closing this PR.

@levb levb closed this Apr 26, 2023
@levb levb deleted the lev-set-conn-handlers branch June 14, 2023 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants