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

Proposer add logging and registration check #546

Merged
merged 1 commit into from
Mar 2, 2022

Conversation

Westlad
Copy link
Contributor

@Westlad Westlad commented Feb 28, 2022

This PR fixes a bug in the dev environment (or indeed any environment) whereby restarting the proposer application will fail if it has already been registered with optimist. It does that by requesting a list of registered proposers from optimist and only attempting to register if its address is not on the list.

This PR also adds some logging to the nf3 class. Specifically, opening, closing, messaging and errors are all now logged for the proposer websocket, although actual log details are limited because of the way websockets work.

Note that to add the logger, some the rather brittle common-files psuedo-packaging has been removed from proposer and user-local as this did not work well when importing both common-files and cli. It is unnecessary in any case now the Dockerfiles have been moved to the repository root.

The e2e test script runners in package.json have been modified to disable logging when these are run, otherwise the test output is spammed with log data.

To test, run the ping-pong tests (see the readme in tests/ping-pong). You will see the new log output as the test proceeds. If you wish to test duplicate registration, the easiest way is to add await nf3.registerProposer(); at line 24 of index.mjs. This will force the proposer to attempt to register twice. A warning will be logged and the second attempt will not proceed.

Copy link
Contributor

@signorecello signorecello left a comment

Choose a reason for hiding this comment

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

Just a shout out so we have consistent env variable locations

package.json Show resolved Hide resolved
@Westlad Westlad merged commit ffef627 into master Mar 2, 2022
@Westlad Westlad deleted the westlad/proposer-logging branch March 2, 2022 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants