search

launchdarkly / electron-client-sdk

LaunchDarkly Client-side SDK for Electron
Other
4 stars 9 forks source link

Streaming connection error causes endless logs while offline #38

Open SLOSK opened 2 years ago

SLOSK commented 2 years ago

Is this a support request? no Describe the bug When offline LD tries to connect to stream every second and prints two logs at the info level each time. We don’t mind that LD continuously tries to connect, but we would prefer a way to not print the errors every second as it spams our log files. I believe it is a similar issue to the one described (and solved) in this thread: launchdarkly/react-client-sdk#2

To reproduce

Expected behavior That after the first warn log the info logs are printed once, not at all, or less frequently. In thread https://github.com/launchdarkly/react-client-sdk/issues/2 they opt to print them once per disconnect.

Logs

{"level":"warn","message":"['Error on stream connection: getaddrinfo ENOTFOUND clientstream.launchdarkly.com, will continue retrying every 1000 milliseconds.']"}
{"level":"info", "message":"[ [ 'Opening stream connection to https://clientstream.launchdarkly.com/eval/***']]"}
{"level":"info","message":"[ [ 'Closing stream connection' ] ]"}

SDK version 1.6.3

Language version, developer tools Electron: 13.6.3

OS/platform Windows 10, MacOS(12.3)

kinyoklion commented 2 years ago

Hello @SLOSK,

I believe that the referenced item only relates to the warning, which should now be printed once, and then only again after a successful connection. The info level messages, which you are seeing would remain.

For this case it might be appropriate to only include logs of warning or higher instead of including the informative logs.

The logging level of the SDK can be adjusted, and also customized. https://docs.launchdarkly.com/sdk/features/logging/?q=logging#electron

If you are using the default logger currently you can view its configuration here: https://github.com/launchdarkly/electron-client-sdk/blob/master/src/index.js#:~:text=function-,createDefaultLogger(),-%7B

Thank you, Ryan

SLOSK commented 2 years ago

Hi @kinyoklion,

There are other logs at the info level we would like to receive so changing the log level isn't an option. Some users are offline for extended periods of time or choose to not go online at all and in that case two logs per second which all say the exact same thing is a bit unnecessary.

I appreciate your response,

Jude

kinyoklion commented 2 years ago

Hey @SLOSK,

That makes sense.

We are working on a more general purpose handling of background handling for the JS SDK, and then that may provide a way for us to handle this more gracefully as well. Selectively enabling/disabling the streaming messages when you may or may not care.

In the interim, because you have a custom logger, you could filter those message specifically. I know that it is less than ideal though.

Thank you, Ryan

SLOSK commented 1 year ago

Hello again Ryan ! I made myself a note to check back in on this --

"We are working on a more general purpose handling of background handling for the JS SDK"

Any news ?

kinyoklion commented 1 year ago

Hey @SLOSK,

We were working on this as part of the change over to not using unload events (released in js-client-sdk 3.1). I ended up decoupling these two things though, so we went ahead with using visibility events, but didn't incorporate this into it. Currently the Electron SDK needs a major update to be better compatible with recent versions of Electron and also to make it less prone to usage errors. This will be one of the things we are considering when we work on that update.

Thanks, Ryan