napajs icon indicating copy to clipboard operation
napajs copied to clipboard

transport.register doesn't work

Open nveenjain opened this issue 7 years ago • 5 comments

I was using transport.register to register a simple class as transportable, but it doesn't work without a decorator, we require to implement transport in a production ready code, and most of our team is skeptic about using decorators in JS before they become common use. The code which produced error was :-

var napa = require('napajs');
var path = require("path");
var {AutoTransportable} = require(path.join(require.resolve("napajs"),"../","transport","transportable"));
var zone1 = napa.zone.create('zone1', { workers: 4 });

class z extends AutoTransportable{
    method1(){
        return "It Worked!";
    }
};
napa.transport.register(z);
console.log(napa.transport.isTransportable(new z()));

Expected result should return true

Current Behaviour throws error saying:- Error: Class "z" doesn't implement cid(), did you forget put @cid decorator before class declaration?

nveenjain avatar Mar 26 '18 19:03 nveenjain

TL;DR

  • A cid is required to define a transportable class.
    • (typescript) Apply decorator @cid() to the class definition.
    • (javascript) [1] Add property _cid to the class constructor. Its value should be a global unique string. [2] call transport.register to register the class.
  • Put the class definition in a separate file and require() the file in all javascript context.

This error msg looks confusing. What it is actually saying is, if you are using typescript, simply adding a cid() decorator to the class definition will resolve this error.

To resolve the error in typescript, simply apply the decorator:

@cid() // adding decorator cid()
class z extends AutoTransportable{
    method1(){
        return "It Worked!";
    }
};

If you are not using typescript, the code need to be modified like this:

class z extends AutoTransportable{
    method1(){
        return "It Worked!";
    }
};
z._cid = "a-unique-name-string"; // adding property _cid to the class constructor
napa.transport.register(z); // register the class

This will resolve the reported error. However, if you try to pass it in execute() like this:

var z1 = new z();

zone1.execute((z1) => {
    console.log(z1.method1());
}, [z1]);

you will find it end up with another error:

Error: Unrecognized Constructor ID (cid) "a-unique-name-string". Please ensure @cid is applied on the class or transport.register is called on the class.

Well.. This may be another confusing error message because we are pretty sure we did register it just now. What it is actually saying is, yes we called napa.transport.register(z); in our main thread BUT NOT in the 4 threads of Zone1. We need to broadcast those code to the zone before transporting an object of type z:

zone1.broadcast("\
    class z extends AutoTransportable{ \
        method1(){ \
            return \"It Worked!\"; \
        } \
    }; \
    z._cid = \"a-unique-name-string\"; \
    napa.transport.register(z); \
");

Although this works, it looks definitely bad code. A better idea is to use a separated file for the class definition:

// z.js
var napa = require('napajs');
var path = require("path");
var { AutoTransportable } = require(path.join(require.resolve("napajs"), "../", "transport", "transportable"));

class z extends AutoTransportable{
    method1(){
        return "It Worked!";
    }
};
z._cid = "a-unique-name-string"; // adding property _cid to the class constructor
napa.transport.register(z); // register the class

module.exports = z;

Then we use it from our main.js:

// main.js
var napa = require('napajs');
var zone1 = napa.zone.create('zone1', { workers: 4 });

var z = require("./z");

var z1 = new z();
zone1.broadcast(() => {
    require("./z"); // broadcast this line so that class z will be registered in every thread in this zone.
});
zone1.execute((z1) => {
    console.log(z1.method1());
}, [z1]);
// output: It Worked!

:v::v::v:

fs-eire avatar Mar 27 '18 00:03 fs-eire

Wow, thanks @fs-eire for replying and resolving the doubt. Yes, you are right that this error message looks confusing, also, wouldn't it be better if it was documented in docs already? I'm ready to send a PR if you agree. Thanks for awesome work you and entire napajs team is doing.

nveenjain avatar Mar 27 '18 06:03 nveenjain

@nveenjain sure you are welcome to send the PR if you got it ready!

fs-eire avatar Mar 28 '18 23:03 fs-eire

@fs-eire , while i was updating it's docs, i encountered something weird. while passing z1 as you instructed above, z constructor is called twice. Once in main thread, and other in other thread and that too without any argument which was supplied previously, Now the following code doesn't work

//class.js
const napa  = require("napajs");
var path = require("path");

var { AutoTransportable } = require(path.join(require.resolve("napajs"), "../", "transport", "transportable"));

class z extends AutoTransportable {
    constructor(source){
        super();
        this.x = source;
    }
    source(){
       console.log(this.x);
    }
};
z._cid = "This_is_class_x";
napa.transport.register(z);
module.exports = z;
// main.js
var napa = require('napajs');
var zone1 = napa.zone.create('zone1', { workers: 4 });
const z  = require("./class");
zone1.broadcast(()=>{require('./class')});
zone1.execute((z1)=>{
    z1.source();
}, [new z("Hey")]);

Now this.x doesn't work. Any workaround you would like to suggest?

nveenjain avatar Apr 02 '18 15:04 nveenjain

After some investigate, I found this is a bug. I've done a fix in PR #216.

It need several days before a new release of Napa.js. If you want to unblock yourself ASAP, you can declare class z to inherit from TransportableObject instead of AutoTransportable. like this:

class z extends TransportableObject {
    constructor(source){
        super();
        this.x = source;
    }
    source(){
       ...
    }

    save(payload, context) {
        payload.x = napa.transport.marshallTransform(this.x, context);
    }
    load(payload, context) {
        this.x = payload.x;
    }
};

fs-eire avatar Apr 04 '18 06:04 fs-eire