search

log4js-node / smtp

SMTP Appender for log4js-node
Apache License 2.0
14 stars 3 forks source link

smtpAppender error unhandled when no network present #19

Closed smartaz closed 2 years ago

smartaz commented 9 years ago

Is it expected for the log4js to quit the process with the following:

log4js.smtpAppender - Error happened { [Error: getaddrinfo ENOTFOUND smtp.gmail.com] code: 'ENOTFOUND', errno: 'ENOTFOUND', syscall: 'getaddrinfo', hostname: 'smtp.gmail.com' }

nomiddlename commented 7 years ago

Related to log4js-node/log4js-node#575 - needs a standard way of handling appender errors

pharapiak commented 5 years ago

I have a similar situation with the TCP appender. I'm using it as an optional appender, in parallel with console, for developers working on a multi-server application. They might be running it, or maybe not. Or they may change the log4js configuration on the tcp-server, which would trigger a restart, and temporarily broken connections.

It would be nice to have an option to let the logging apps keep on running in the face of TCP connection failures.

I was thinking along the lines of a callback that was passed the appender, which could decide whether to ignore the error, throw it, or maybe retry the write operation, or disable the appender.

lamweili commented 2 years ago

@pharapiak

lamweili commented 2 years ago

The original issue of the stmpAppender might need to be transferred to https://github.com/log4js-node/smtp. @nomiddlename You reckon?

nomiddlename commented 2 years ago

@peteriman good call.

lamweili commented 2 years ago

Background from Node.js

If an EventEmitter does not have at least one listener registered for the 'error' event, and an 'error' event is emitted, the error is thrown, a stack trace is printed, and the Node.js process exits. (src: https://nodejs.org/docs/latest/api/events.html#error-events)

Proposed fix:

A fix would be to add an error listener to the EventEmitter. Can refer to https://github.com/log4js-node/log4js-node/pull/1089/files or below:

https://github.com/log4js-node/smtp/blob/2b122dd92492cfb6019dbc499b0620904dcacf19/lib/index.js#L40

=L40 const transport = mailer.createTransport(getTransportOptions(config));
+L41 transport.on('error', (e) => {}); // replace with code for error handling

Reference details:

Due to nodemailer's mailer.createTransport returning \<Mail> (which is an EventEmitter). Need to add a listener to its 'error' event (bubbled from this.transporter) to prevent Node.js exit.

class Mail extends EventEmitter {

https://github.com/nodemailer/nodemailer/blob/master/lib/mailer/index.js#L23

            // transporter errors
            this.transporter.on('error', err => {
                this.logger.error(
                    {
                        err,
                        tnx: 'transport'
                    },
                    'Transport Error: %s',
                    err.message
                );
                this.emit('error', err);
            });

https://github.com/nodemailer/nodemailer/blob/master/lib/mailer/index.js#L73-L84