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

Flush logger before exiting process #10905

Merged
merged 5 commits into from
Jan 26, 2022
Merged

Conversation

joshuaspence
Copy link
Contributor

I am using the file logger without the console logger and if Z2M is unable to start then nothing is logged because process.exit(1) is calling and WInston doesn't flush async logs when this happens (see winstonjs/winston#1629).

With console logging enabled:

Zigbee2MQTT:info  2022-01-20 05:01:21: Logging to console and directory: '/app/data' filename: z2m.log
Zigbee2MQTT:info  2022-01-20 05:01:21: Starting Zigbee2MQTT version 1.22.2-dev (commit #)
Zigbee2MQTT:info  2022-01-20 05:01:21: Starting zigbee-herdsman (0.14.1)
Zigbee2MQTT:error 2022-01-20 05:01:21: Error while starting zigbee-herdsman
Zigbee2MQTT:error 2022-01-20 05:01:21: Failed to start zigbee
Zigbee2MQTT:error 2022-01-20 05:01:21: Check https://www.zigbee2mqtt.io/guide/installation/20_zigbee2mqtt-fails-to-start.html for possible solutions
Zigbee2MQTT:error 2022-01-20 05:01:21: Exiting...
Zigbee2MQTT:error 2022-01-20 05:01:21: Error: Error while opening serialport 'Error: Error: No such file or directory, cannot open /dev/ttyACM0'
    at SerialPort.<anonymous> (/app/node_modules/zigbee-herdsman/src/adapter/z-stack/znp/znp.ts:146:28)
    at SerialPort._error (/app/node_modules/zigbee-herdsman/node_modules/@serialport/stream/lib/index.js:198:14)
    at /app/node_modules/zigbee-herdsman/node_modules/@serialport/stream/lib/index.js:242:12

With file logging:

info  2022-01-20 05:03:36: Logging to directory: '/app/data' filename: z2m.log
info  2022-01-20 05:03:36: Starting Zigbee2MQTT version 1.22.2-dev (commit #)
info  2022-01-20 05:03:36: Starting zigbee-herdsman (0.14.1)
error 2022-01-20 05:03:36: Error while starting zigbee-herdsman

// Workaround for https://github.com/winstonjs/winston/issues/1629
await new Promise<void>((resolve, reject) => {
logger.winston.on('finish', () => {
setTimeout(() => resolve(), 1000);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, this will always increase the shutdown sequence by 1 second, could this be decreased?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is my understanding as well. Possibly it could be decreased but since winston writing to file is async there's no real safe value. The shorter the timeout the more likely that the logs won't be written to disk.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried a few different options that wouldn't add unnecessary delay to shutdown but unfortunately wasn't able to get anything else to work

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just checked, setTimeout(() => resolve(), 10); also fixes it, could you also test this?

@joshuaspence joshuaspence force-pushed the flush-logger branch 2 times, most recently from 598ea23 to bcf09cc Compare January 21, 2022 01:50
@Koenkk Koenkk enabled auto-merge (squash) January 26, 2022 15:56
@Koenkk Koenkk merged commit 1a70b80 into Koenkk:dev Jan 26, 2022
@Koenkk
Copy link
Owner

Koenkk commented Jan 26, 2022

nice solution, thanks!

@joshuaspence joshuaspence deleted the flush-logger branch January 26, 2022 22:59
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.

2 participants