CODEWITHSUNDEEP
javascriptjquerysetinterval
Toni Michel Caubet
Toni Michel Caubet

Reputation: 20173

jQuery polling chat - duplicate entries

Here is my scenario:

This is the script that generates the chat content:

function refresh_chat(){
    var last = $('.conversation li:not(.fake):last').data('id');
    $.post('includes/router.php', {
        task: 'update_conversation',
        id: '<?=$_GET['conversationid']?>',
        last: last
    }, function (data, response) {
        var recibidas = $(data).find('li');

        /* IF there are new entries */
        if (recibidas.length > 0) {
            /* Remove all fake entries */
            $('.conversation li.fake').remove();

            /* Append new entries */
            $('.conversation').append($(data).filter('.notifications').html());

            /* If this new entries are not unread, 
               remove the unread to the previous ones*/
            if(!$(data).find('li:last').hasClass('unread')) {
                $('.conversation li.unread').removeClass('unread');
            }
        }
    });
}

var t = setInterval(function () {   
    refresh_chat();
}, 3000);

And this is how I add a new entry when the user types it:

$('body').on('submit', '.send_message_form_conversation', function(e) {
    e.preventDefault();
    var id_to = $(this).find('#id_to').val();
    var msj = $(this).find('#msj').val();
    if (msj.length >= 2) {
        $(this).find('#msj').val('');
        $(this).find('.nicEdit-main').html('');
        //alert(id_to);
        $('.conversation').append(
            '<li class="unread fake">' +
             '<div class="avatar">' +
              '<a href="index.php?userid=<?=sesion()?>">' +
               '<img alt="" src="<?=$_SESSION['avatar']?>">' +
              '</a>' +
             '</div>' +
             '<div class="txt">' +
              '<a class="userName" href="index.php?userid=<?=sesion()?>">' + 
               '<?=$_SESSION['alias']?> -- ' +
               '<span class="date">' +
                "<?=get_texto_clave('ahora mismo')?>" +
               '</span>' +
              '</a>
             '<span class="msj">' + msj + '</span>' + 
            '</div>' +
            '<span data-id="47" class="close">X</span>' + 
           '</li>');

        $.post('includes/msj.php?', {
            task  : 'post_message',
            id_to : id_to,
            msj   : msj
        }, function (data, response) {
            $(".conversation").scrollTop($(".conversation")[0].scrollHeight);
        });
    } else {
        $(this).parent().effect("shake", { times:0, distance: 3 }, 200);
    }                       
});

As you can see, the <li> items may have two classes: .fake (which means that this item is a preview of what the user just submitted, and has been appended by js) or .unread (this means that the receiver has just received the message)

The thing I'm struggling with is that sometimes I start seeing some duplicated entries (only displayed, though - they are not duplicated in the database). I am guessing that there is something wrong with my interval?

What could be causing this? (I just keep reading it but I can't find anything weird...)

PD: basically, some messages are being showed more than once :S

-EDIT-

$q = "SELECT * FROM pms " .
     "WHERE ((id_to = $id and id_from = " . sesion() . ") OR " .
     "       (id_from = $id and id_to = " . sesion() . ")) " .
     "AND (id > $from) " .
     "ORDER by fecha ASC " . $limit;

This query is the one used in the $.post() request where $from is last in the JavaScript parameter (wich represents the last message shown to the user)

Upvotes: 3

Views: 1167

Answers (8)

Louis Ricci
Louis Ricci

Reputation: 21106

A few points, the 'unread' class doesn't seem to do much. You're auto refreshing anyways, when is a private message read or unread? If someone closes the chat and goes back later should they get all of their messages or just the newest ones? Just things to think about...

You bug is likely because you use setInterval and resend the same request here's some code that corrects the issue. Notice the use of the "last_id" variable to stop repeated requests, as well as, calling setTimeout only after processing has completed.

I would suggest you use long polling or comet for your chat. Comet, long polling with jquery tutorial If you want to be really cutting edge go with HTML5 WebSockets, but you'll have to do some server configuration.

function refresh_chat(){
    var last = $('.conversation li:not(.fake):last').data('id');
    if(last_id == last) {
        /* this request was already successfully processed */
        /* wait some more and try again */
        if(auto_refresh) window.setTimeout(refresh_chat, 3000);
        return;
    }
    $.ajax({
    type: 'POST',
    url: 'includes/router.php',
    data: { task: 'update_conversation',id: '<?=$_GET['conversationid']?>',last: last },
    success: function(data) {
        var recibidas = $(data).find('li');
        /* IF there are new entries */
        if (recibidas.length > 0) {
            last_id = last;
            /* Remove all fake entries */
            $('.conversation li.fake').remove();
            /* Append new entries */
            $('.conversation').append($(data).filter('.notifications').html());
        }
        //
        if(auto_refresh) window.setTimeout(refresh_chat, 3000);
    },
    error: function(jqXHR, textStatus, errorThrown) {
        console.log("Ajax Error: " + (textStatus||""));
        //
        if(auto_refresh) window.setTimeout(refresh_chat, 3000);
    }
    });
}
var auto_refresh = true;
var last_id = -1;
refresh_chat();

Upvotes: 0

Aitor Calderon
Aitor Calderon

Reputation: 773

As @zch points out, this problem may raise with your actual method.

I'd suggest 2 approaches:

1.- Just before calling refresh_chat(), abort the last post to server. By doing this, if the response didn't make it to the browser (or the server didn't answer at all) after those 3 seconds you discard your last try, and send a request again. This is not the best approach as server resources could be wasted with responses that will never be taken.

2.- Try to make a circular counter (like from 0 to 10), increment and send it with each request, and make the server send it back, then, discard any non coincident response. Something like this:

Request with 0 -> Response with 0
Request with 1 -> Response with 1
Request with 2 -> (no response)
Request with 3 -> Response with 2 (Discarded as it doesn't match)
Request with 4 -> (no response yet)
           -> Response with 3 (Discarded as 4 is already sent)
           -> Response with 4 (Accepted, actual Request was with 4)

Hope you find this useful.

Upvotes: 0

larsson
larsson

Reputation: 11

The problem you are having is related to the fact that setInterval will schedule a fresh execution of your refresh_chat function every 3000 milliseconds, regardless of whether there is a current execution going on or not.

What you should do instead is to simply call refresh_chat once yourself, and then at the very bottom of the internal callback function (the one that is called when there is data coming back from the server), you add a setTimeout(refresh_chat, 3000).

This will make sure that the client does not attempt to schedule a new invocation of the refresh_chat function until you know that it has finished its work.

Think of it as a delayed recursive call of sorts (although, technically, it isn't one, since the call stack is not ever-increasing).

Upvotes: 0

Cvetan Nenov
Cvetan Nenov

Reputation: 121

When the problem appears , check what is your returned data from the server ( i.e. ajax response body ) to see whether "old" data is there too. If this is the case you have to debug your query.

Also , your DB query is VERY vulnerable to SQL injection attacks http://en.wikipedia.org/wiki/SQL_injection (for example supply last with the value of : x'; DROP TABLE pms; -- ) 

 $q = "SELECT * FROM pms " .
 "WHERE ((id_to = $id and id_from = " . sesion() . ") OR " .
 "       (id_from = $id and id_to = " . sesion() . ")) " .
 "AND (id > $from) " .
 "ORDER by fecha ASC " . $limit;

Finally, the polling doesn't seem the best way to achieve a chat app. Maybe a combination of Sockets and polling ( if the browser doesn't support sockets) will work better. Best C

Upvotes: 1

catearcher
catearcher

Reputation: 51

Give Socket.io a try! There won't be any noticeable lags even if you send the message to the server, parse and transform it there and send it back, thus making a preview unnecessary.

Using Ajax and setTimeout for a chat application will cause you nothing but pain.

Upvotes: 1

pregmatch
pregmatch

Reputation: 2657

this ajax chat is not a solution at all. You need to read this book. If you do not have jabber server I will give you acces to mine (user register update etc). Read the book and then contact me. It is XMPP + Strophe library chat (that what google and facebook are using)! So it is better to start over and learn something new then do fix error in ajax chat!

Upvotes: -1

zch
zch

Reputation: 15278

Think about scenario:

  1. refresh_chat() - request sent to server
  2. 3 seconds pass
  3. refresh_chat() - identical request sent to server
  4. Response to first received. New entries added.
  5. Response to second received. The same entries added again.

The simplest way to solve this is to remove setInterval, and add setTimeout after the response is processed.

Upvotes: 3

davehale23
davehale23

Reputation: 4372

I think it's because you are only 'hiding' the fake entries if there are new ones to be shown:

if(recibidas.length>0){
    $('.conversation li.fake').remove();

I recommend taking this out of that if statement. Seems like you should have the remove() after you post a new message, like inside your function(data,response){ of the submit function maybe.

Upvotes: 0

Related Questions