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.

Sign up for GitHub

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

Jump to bottom

Tail is not updated in real time / requires manual 'refresh' using Node >=10 #82

Closed
meatwallace opened this issue Sep 18, 2019 · 1 comment · Fixed by #84
Closed

Tail is not updated in real time / requires manual 'refresh' using Node >=10 #82

meatwallace opened this issue Sep 18, 2019 · 1 comment · Fixed by #84
Assignees
@wearhere
Labels
bug Something isn't working

Comments

@meatwallace
Copy link

After upgrading to Node 10.16.x Custody does not seem to tail the log, only outputting the current state of the log when it's re-rendered. This effects both process logs and the 'main' combined log.

This can be reproduced by viewing a service's log (or the main log) and performing an action that causes the log to update. No update will be reflected, however simply pressing tab twice (swapping between process <-> main log) will update the log to reflect the expected state.

I was unable to install @custody/cli on Node 9.x, earlier versions of Node 10.x, and Node 12.x due to node-gyp issues with the kexec dependency, so I can't confirm when the issue was introduced. However, Custody works fine with Node 8.9.3.

I've tested this inside and outside of a tmux session in both Kitty and Mac's Terminal.app and been able to reliably reproduce it in all cases.
@wearhere wearhere self-assigned this Sep 19, 2019
@wearhere wearhere added the bug Something isn't working label Sep 19, 2019
@wearhere
Copy link
Contributor

wearhere commented Sep 19, 2019 •
edited
Loading

Ok, the issue here is that we use node-tail and in turn fs.watch to determine when log files have been updated, but fs.watch does not consistently emit 'change' events on Node 10.16.0, possibly due to nodejs/node#29460. fs.watchFile still works, but is less efficient, per the docs, so I'll switch to that only if necessary and also suggest that users run custody under an older version of Node. Also filing a follow-up issue to track reverting this change.

@wearhere wearhere mentioned this issue Sep 19, 2019
Open
wearhere added a commit that referenced this issue Sep 19, 2019
@wearhere
Fix logfile tailing under Node 10.16.0
5caea0c 
`fs.watch` does not consistently emit `'change'` events on Node 10.16.0,
possibly due to nodejs/node#29460. `fs.watchFile`
still works, but is less efficient, per the docs, so fall back to that only if necessary and also suggest that users run custody under an older version of Node.

I explored several Node file-watching libraries (e.g. chokidar, sane) to try to
find a more performant solution for Node 10.16.0 and it seems that they pretty
much all fall back to `fs.watch` / `fs.watchFile`, unfortunately. sane offers
the option to use watchman, but that requires installing a daemon so is a
non-starter.

Fixes #82.

#83 tracks reverting this when
the underlying Node issue has been fixed.

Tested that log files update both when running custody under v8.9.3 and
v10.16.0, and that `node-tail` only uses `fs.watchFile` in the latter.
@wearhere wearhere mentioned this issue Sep 19, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@wearhere wearhere

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix logfile tailing under Node 10.16.0 mixmaxhq/custody
2 participants
@wearhere @meatwallace