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

Bug in nested seneca instance calls #165

Open
davide-talesco opened this issue May 21, 2018 · 2 comments
Open

Bug in nested seneca instance calls #165

davide-talesco opened this issue May 21, 2018 · 2 comments
Assignees
Labels

Comments

@davide-talesco
Copy link

davide-talesco commented May 21, 2018

Hi all,

if you have a seneca instance (seneca1) calling another seneca instance (seneca2) which calls another seneca instance (seneca3), (sounds long but it should be pretty legit) if the last one, seneca3, return an error, its caller (seneca2) will throw the error: Error:Converting circular structure to JSON and will never reply about the error to its caller seneca2

const Boom = require('boom');
const seneca1 = require('seneca')({tag: 'seneca1'});
const seneca2 = require('seneca')({tag: 'seneca2'});
const seneca3 = require('seneca')({tag: 'seneca3'});

seneca1
  .client({port:4000, pin: 'cmd:test1'})
  .add('cmd:test', function(msg, reply){
    const seneca = this;

    seneca.act('cmd:test1' , function(err, res){
      return reply(err, res);
    })
  })
  .listen(3000);

seneca2
  .client({port:5000, pin: 'cmd:test2'})
  .add('cmd:test1', function(msg, reply){
    const seneca = this;

    seneca.act('cmd:test2' , function(err, res){
      return reply(err, res);
    })
  })
  .listen(4000);

seneca3
  .add('cmd:test2', function(msg, reply){
    const err = new Boom('this error will cause seneca2 to throw a JSON stringify circular error @ seneca-transport http.js')
    return reply(err);
  })
  .listen(5000);

setTimeout(function(){
  seneca1.act('cmd:test', function(err){
    // this will time out because line 23 will never be called
    //console.log(err);
  })
}, 3000);

the error happen here:

outJson = transportUtil.stringifyJSON(seneca, 'listen-web', out.error)

and this is the stacktrace:

TypeError: Converting circular structure to JSON
at JSON.stringify ()
at module.exports.internals.Utils.internals.Utils.stringifyJSON (/Users/davide_talesco/development/crap/seneca/errorHandling3/node_modules/seneca-transport/lib/transport-utils.js:563:17)
at Object.internals.sendResponse (/Users/davide_talesco/development/crap/seneca/errorHandling3/node_modules/seneca-transport/lib/http.js:256:29)
at /Users/davide_talesco/development/crap/seneca/errorHandling3/node_modules/seneca-transport/lib/http.js:242:15
at Seneca. (/Users/davide_talesco/development/crap/seneca/errorHandling3/node_modules/seneca-transport/lib/transport-utils.js:260:7)
at Object.intern.handle_reply (/Users/davide_talesco/development/crap/seneca/errorHandling3/node_modules/seneca/seneca.js:1311:13)
at Seneca.action_reply (/Users/davide_talesco/development/crap/seneca/errorHandling3/node_modules/seneca/seneca.js:906:24)
at Seneca. (/Users/davide_talesco/development/crap/seneca/errorHandling3/root.js:23:14)
at Object.intern.handle_reply (/Users/davide_talesco/development/crap/seneca/errorHandling3/node_modules/seneca/seneca.js:1311:13)
at Seneca.action_reply (/Users/davide_talesco/development/crap/seneca/errorHandling3/node_modules/seneca/seneca.js:906:24) {stack: TypeError: Converting circular structure to JSON
…orHandling3/node_modules/seneca/seneca.js:906:24),
message: Converting circular structure to JSON}

@rjrodger Any idea when someone will take this project back to life?
It is a pity such a good project and no one to taking care of it.

I would be very happy to help, but the seneca community, seems non existent and feels very hard to contribute.
I am not too sure if seneca is still being developed at all and I wonder if anyone is still actively using it.

Davide

@rjrodger
Copy link
Collaborator

I have incorporated this test into the main seneca repo - thanks!
However, it only passes on the new transport implementation (which is still in beta until seneca 4).
If you are still up for helping, please update seneca-transport to fix this - I am happy to grant access etc as needed.
Yes, seneca is in use - running my new startup! https://github.com/voxgig

@davide-talesco
Copy link
Author

The problem its actually in the transport-utils code:

return JSON.stringify(obj)

obj includes the incomingResponse object at this paths:

  • obj.data.response;
  • obj.orig.data.response;

and JSON.stringify cannot serialize it because of its cyclical structures.

I simply used a JSON.stringify replacer to find and filter (thus causing data loss) any cyclic references (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Cyclic_object_value#Examples) though I am not too sure the impact from a performance perspective...

A safer option, with no data loss, would be to use something like this: https://github.com/douglascrockford/JSON-js to safely serialize and deserialize a circular structure to JSON without data loss.

a simpler and more efficient solution, could be to just remove the IncomingMessage from the response object?

what do you think? how come the new transport is not affected by this issue?

I added the JSON replacer here https://github.com/davide-talesco/seneca-transport if you want to have a look.

All tests pass, but I am not too sure if internals.Utils.prototype.stringifyJSON is used anywhere else where the inevitable data loss of the replacer might cause unexpected side effect.

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

No branches or pull requests

2 participants