Adding error object to logger in sentinel/server.js

[robin-murphy1]

What feature do you want to improve?

In the Cht-Core/Sentinel module the server.js file has a error block that prints out a text message but does not include the error message.

//filename: sentinel/server.js

24 request({ url, json: true }, (err, response, body) => {
25         if (err) {
26           logger.info('Waiting for API to be ready...');
27           return setTimeout(() => waitLoop(), 10 * 1000);
        }

It took some time to realize that Sentinel wasn't connecting and figure out that it was failing to connect to the API. Adding the error object to the log message helped in troubleshooting the issue quickly.

Describe the improvement you'd like

Modify the sentinel/server.js file to include the error object in the log message.

//filename: sentinel/server.js

24 request({ url, json: true }, (err, response, body) => {
25         if (err) {
26           logger.warn('Waiting for API to be ready...', err);
27           return setTimeout(() => waitLoop(), 10 * 1000);
        }

Describe alternatives you've considered

None

Additional context

Small change but first time contributor so want to understand the process of contributing to the source base

[garethbowen]

Hi @robin-murphy1 and welcome!

This looks like a nice change and you're most welcome to submit a PR for review. If you're interested here's some documentation about how the process works: https://docs.communityhealthtoolkit.org/contribute/code/

If you'd like to contribute the fix, the first step is to assign this issue to yourself, then fork the codebase locally, and start working on a Pull Request.

One thing to think about is the impact this may have on the log size if sentinel can't connect for a long time. A couple of options to consider:

1. caching the error and only outputting it if it's different from the previous error, or
2. only printing the error every 10th attempt

Also a slight change to the logging, because we use winston it's better to output the error like so:

logger.warn('Waiting for API to be ready...: %o', err);

You can see other examples like this in that file too.

Please reach out if you have any other questions.

[ElijahBus]

Hi @garethbowen ! What's the status of this issue, did it get resolved ?

I can see that it didn't go further from here, I am ready to move forward with it.

[garethbowen]

@ElijahBus I don't think @robin-murphy1 is working on this so please feel free to pick it up.