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

smtpAppender error unhandled when no network present #19

Closed
smartaz opened this issue Nov 5, 2015 · 6 comments · Fixed by #20
Closed

smtpAppender error unhandled when no network present #19

smartaz opened this issue Nov 5, 2015 · 6 comments · Fixed by #20
Milestone

Comments

@smartaz
Copy link

smartaz commented Nov 5, 2015

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
Copy link
Contributor

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

@pharapiak
Copy link

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
Copy link
Contributor

@pharapiak

  • tcpAppender (#1089) - fixed in log4js@6.4.0

@lamweili
Copy link
Contributor

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

@nomiddlename nomiddlename transferred this issue from log4js-node/log4js-node Jan 20, 2022
@nomiddlename
Copy link
Contributor

@peteriman good call.

@lamweili
Copy link
Contributor

lamweili commented Jan 20, 2022

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:

const transport = mailer.createTransport(getTransportOptions(config));

=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

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 a pull request may close this issue.

4 participants