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

Change connection error type to warning #225

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kuldarim
Copy link

@kuldarim kuldarim commented Oct 6, 2022

While debugging issue with project on which I'm currently working on I have noticed one issue which ends up in node.js running out of memory situation.

My case is pretty simple, there are some situations, when elastic search instance is not reachable and winston-elasticsearch fails to connect to it. To not loose the errors, which occurs in the mean time you can use buffering feature. Which basically collects errors in array and tries to send when to elastic search when connection again becomes stable. This is great, because it does not let you to loose any stack traces.

Winston itself offers functionality to handleExceptions and handleRejections on the node process its currently running on. These options can be configured per transport level. So we end up in situation, if you use winston-elasticsearch as a transport with handleExceptions and handleRejections set to true if connection to elastic search instance will faill it will push those errors to buffer, because if connection fails it executes thiz.transport.emit('warning', err)) which is caught by winston, because handleExceptions and handleRejections are set to true.

This is not the case if you set buffering to false. Because then instead it executes the line 177 in bulk_writer.js
which emits thiz.transport.emit('warning', e) not error.

Basically to reproduce it. You need to setup all props as following:

const transports = [
      new ElasticsearchTransport({
        ensureIndexTemplate: false,
        flushInterval: 2000,
        retryLimit: 3,
        buffering: true,
        // max number of messages in buffer
        bufferLimit: 20,
        handleExceptions: true,
        handleRejections: true,
      }),
    )];
    
  return winston.createLogger({
    level: 'debug',
    format: winston.format.simple(),
    transports,
    // By default, node.js will terminate on uncaught errors. Winston can catch and log these errors.
    // If exitOnError is true (the default), Winston will subsequently end the node process, just like node's default behavior.
    // If exitOnError is set to false, the error will be logged and the process will continue.
    exitOnError: false,
  });

So I'm creating this pull request, to align these too cases and in both buffer endabled and disabled scenarious emit warning instead of error.

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.

1 participant