-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add support for starting a userland NATS server #197
Conversation
It would be great if there was another JSON file in the sample configs section that showed the public NATS configuration |
Should we validate that the provided options for the public NATS connection match the public NATS server config? As it currently stands, the code will allow a public NATS server to be configured and the NATS connection could still connect successfully to a different NATS server. |
Let's not do that yet. We shouldn't assume that the userland NATS server being started has anything do with nex other than its packaging. |
n.log.Error("Failed to start internal NATS server", slog.Any("err", err)) | ||
err = fmt.Errorf("failed to start internal NATS server: %s", err) | ||
// start internal NATS server | ||
_err = n.startInternalNATS() |
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.
this looks a little awkward. The underscore prefix typically indicates that the variable isn't being used, but this variable is used in multiple places.
err = n.api.Start() | ||
if err != nil { | ||
n.log.Error("Failed to start API listener", slog.Any("err", err)) | ||
if err == nil { |
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.
could this just be an else clause to the previous if _err != nil
?
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.
The use of _err
instead of err
is a bit jarring to me, but I don't think it's reason enough to stop the PR
This PR allows users to configure and start a "userland" NATS server when starting a node.
Error handling inside of
node.init()
was not failing fast, resulting in discarded errors. This PR includes a fix to useerrors.Join
which allows the node to gracefully abort initialization usingshutdown()
instead of panicking.Closes #168.