readFile not running inside while loop

Question

I am trying to send the contents of a text file through a socket connection every time the text file updates using Express:

console.log('Server running!');

var express = require('express');
var app = express();
var server = app.listen(3000);
var fs = require("fs");
var x = 0;

app.use(express.static('public'));

var socket = require('socket.io');
var io = socket(server);

io.sockets.on('connection', newConnection);

function newConnection(socket) {
  console.log("New connection: " + socket.id);
  while (true) {
    fs.readFile('song.txt', function(err, data) {
      if (err) throw err;
      console.log(data);
      if (data != x) {
        var songdata = data;
        console.log(songdata);
        io.sockets.emit('update', songdata);
        x = data;
      } else {
        console.log("Song is not different:)");
      }
    })
  }
}

Without the while loop, everything works just fine and I recieve the contents in the seperate client. However, now nothing is happening, no console log of data. This indicates the readFile is suddenly no longer running, why?

Thanks:)

`readFile()` is asynchronous. Async callbacks don't run until your code returns to the main event loop, but you have an infinite while loop so it never returns. — Barmar, Sep 19 '18 at 21:28
Either redesign your code to work asynchronously, or use `readFileSync()`. — Barmar, Sep 19 '18 at 21:30
On another note, there are more efficient ways to watch for changes to a file than to read it in a loop, which will do way more reads than necessary. Node 10 now has `fs.watch`: https://nodejs.org/docs/latest/api/fs.html#fs_fs_watch_filename_options_listener — Jacob, Sep 19 '18 at 21:36
Thanks guys:) I've switched to readFileSync instead, and got it working without the loop. When I put the loop in again, the line `io.sockets.emit()` no longer runs. Is that async too? — jbblackett, Sep 19 '18 at 21:47

jfriend00 · Accepted Answer · 2018-09-20T00:00:57.817

First off, some basics. node.js runs your Javascript as single threaded and thus this is a single threaded server. It can only do one thing with your Javascript at a time. But, if you program it carefully, it can scale really well and do lots of things.

Second off, you pretty much never want to do while (true) in server-side Javascript. That's just going to run forever and never let anything else run on your server. Nothing else.

Third, you are attempting to create a new version of that infinite loop every time a new client connects. That's not a correct design (even if there wasn't an infinite loop). You only need one instance of code checking the file, not N.

Now, if you what you're really trying to do is to "poll" for changes in song.txt and notify the client whenever it changes, you need to pick a reasonable time delta between checks on the file and use a timer. This will check that file every so often and let your server run normally all the rest of the time.

Here's a simple version that polls with setInterval():

console.log('Server code started!');

const express = require('express');
const app = express();
const server = app.listen(3000);
const fs = require("fs");
let lastSongData = 0;

app.use(express.static('public'));

const io = require('socket.io')(server);

// get initial songData for future use
// there will not be any connected clients yet so we don't need to broadcast it
try {
    lastSongData = fs.readFileSync('song.txt');
} catch(e) {
    console.log(e, "\nDidn't find song.txt on server initialization");
}

// here, we create a polling loop that notifies all connected clients 
// any time the song has changed
const pollInterval = 60*1000;   // 60 seconds, ideally it should be longer than this
const pollTimer = setInterval(() => {
    fs.readFile('song.txt', (err, songData) => {
        if (!err && songData !== lastSongData) {
            // notify all connect clients
            console.log("found changed songData");
            io.emit('update', songData);
            lastSongData = songData;
        }
    });
}, pollInterval);

io.sockets.on('connection', socket => {
  console.log("New connection: " + socket.id);
});

If your songData is binary, then you will have to change how you send the data to the client and how you compare the data to the previous data so you are sending and receiving binary data, not string data and so you are comparing buffers, not strings.

Here's are some references on sending binary data with socket.io:

How to send binary data with socket.io?

How to send binary data from a Node.js socket.io server to a browser client?

A little more efficient way to detect changes to the file is to use fs.watch() which should notify you of changes to the file though you will have to thoroughly test it on whatever platform you are running to make sure it works the way you want. The feature has a number of platform caveats (it does not work identically on all platforms), so you have to test it thoroughly on your platform to see if you can use it for what you want.

console.log('Server code started!');

const express = require('express');
const app = express();
const server = app.listen(3000);
const fs = require("fs");
let lastSongData = 0;

app.use(express.static('public'));

const io = require('socket.io')(server);

// get initial songData for future use
// there will not be any connected clients yet so we don't need to broadcast it
try {
    lastSongData = fs.readFileSync('song.txt');
} catch(e) {
    console.log(e, "\nDidn't find song.txt on server initialization");
}

// ask node.js to tell us when song.txt is modified
fs.watch('song.txt', (eventType, filename) => {
    // check the file for all eventTypes
    fs.readFile('song.txt', (err, songData) => {
        if (!err && songData !== lastSongData) {
            // notify all connect clients
            console.log("found changed songData");
            lastSongData = songData;
            io.emit('update', songData);
        }
    });
});

io.sockets.on('connection', socket => {
  console.log("New connection: " + socket.id);
});

It is unclear from your original code if you need to send the songData to each new connection (whether it has recently changed or not).

If so, you can just change your connection event handler to this:

io.sockets.on('connection', socket => {
  console.log("New connection: " + socket.id);
  // send most recent songData to each newly connected client
  if (lastSongData) {
      socket.emit('update', lastSongData);
  }
});

Thanks dude, this is amazing and exactly what I needed. Thank you so much! — jbblackett, Sep 20 '18 at 00:09
@jbblackett - If your songData is binary, you will have to adapt this for sending binary data over socket.io, comparing binary data with previous songData and reading binary data in the client. If it's a string, this should be good. — jfriend00, Sep 20 '18 at 00:14

score 0 · Answer 2 · answered Sep 19 '18 at 22:40

Continuously reading the file to detect changes is not a good idea. Instead you should use fs.watch(filename[, options][, listener]) to notify you when the file has changed. When a new socket connects only that socket should have the content broadcast to them, sending it to every client is redundant.

io.sockets.on('connection', newConnection);

var filename = 'song.txt';

function update(socket) {
    fs.readFile(filename, function (err, data) {
        if (err) throw err;
        socket.emit('update', data);
    });
}

function newConnection(socket) {
    console.log("New connection: " + socket.id);
    update(socket); // Read and send to just this socket
}

fs.watch(filename, function () {
    console.log("File changed");
    update(io.sockets); // Read and send to all sockets.
});

readFile not running inside while loop

2 Answers2