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

PubSub triggers [RangeError: Maximum call stack size exceeded] in 0.11.0-rc10 #2636

Closed
Skullbock opened this issue Feb 10, 2015 · 8 comments
Closed

Comments

@Skullbock
Copy link

I've upgraded my sails application to 0.11.0-rc10 today, and it works perfectly, with a very smooth update.

However it seems that there is a small issue with the upgrade to socket.io 1.0 because when i try to manually do a "publishCreate" on a collection without specifying which socket to omit from the socket message, it triggers a [RangeError: Maximum call stack size exceeded] error.

From my debugs, it triggers into the new socket hook, when it calls

// Otherwise broadcast to everyone in the room.
app.io.sockets.in(roomName).emit(eventName, data);

Of course if i specify a socket to omit, it works because it calls:

socketToOmit.broadcast.to(roomName).emit(eventName, data);

Maybe i'm doing something wrong? My call is:

MyCollection.publishCreate(myInstance);

Thanks!

@sgress454
Copy link
Member

@Skullbock interesting, thanks for the heads-up. If it was working before it should be working now, so I don't think you're doing anything wrong. We'll look into it!

@Skullbock
Copy link
Author

Thanks! If you need anything I'm here to help

@nanopx
Copy link

nanopx commented Feb 12, 2015

I've also upgraded sails to 0.11.0-rc10 recently.

This error seems to happen when you include your model inside the data, i.e:

Model.find(id).exec(function(err, model){
  Model.message(id, {
     model: model
  });
});

For now, you could deep clone your model like this.

Model.find(id).exec(function(err, model){
  Model.message(id, {
    model: _.clone(model, true)
    // or below,  if you have toJSON() method overridden inside your model
    // model: _.clone(model.toJSON(), true)
  });
});

Hope this helps.

@Skullbock
Copy link
Author

Thanks for the tip! i can see why it could be that.... Ok i'll fix it this way in the meantime, maybe this could be fixed also in the pubsub hook in the same way?

@erkstruwe
Copy link

I'm glad I finally found this issue description. Thanks @nanopx , your workaround is a good one. We should definitely put this into the pubsub hook. This can break whole applications (as it did to me... :-D).
BTW: This is also in v0.11.0

@Skullbock
Copy link
Author

Just to follow up on this:

From my recent tests, the publishUpdate crashes too, but for a different reason:

// If we have the pubsub hook, use the Model's publish method
      // to notify all subscribers about the update.
      if (req._sails.hooks.pubsub) {
        if (req.isSocket) { Model.subscribe(req, records); }
        Model.publishUpdate(pk, _.cloneDeep(values), !req.options.mirror && req, {
          previous: matchingRecord.toJSON()
        });
      }

and the line making it crash is the previous
I fixed it like this:

   // If we have the pubsub hook, use the Model's publish method
      // to notify all subscribers about the update.
      if (req._sails.hooks.pubsub) {
        if (req.isSocket) { Model.subscribe(req, records); }
        Model.publishUpdate(pk, _.clone(values, true), !req.options.mirror && req, {
          previous: _.clone(matchingRecord.toJSON(), true)
        });
      }

let me know @nanopx and @mikermcneil if you find anything else on this ;)

@tobalsgithub
Copy link

I'm seeing something similar, but with the blueprinted destroy routes. When we have sockets.adapter set like so:

sockets: {
  adapter: 'socket.io-redis'
}

we hit a maximum call stack error when hitting the delete blueprint route.
Uncaught Maximum call stack size exceeded

This isn't a big deal for us since we're moving away from the blueprinted routes, just thought I'd add to the examples.

erkstruwe added a commit to erkstruwe/sails that referenced this issue Jul 24, 2015
@Nishchit14
Copy link

Getting same error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants