Skip to content
This repository was archived by the owner on Jun 13, 2024. It is now read-only.

transport.register doesn't work #209

Open
nveenjain opened this issue Mar 26, 2018 · 5 comments
Open

transport.register doesn't work #209

nveenjain opened this issue Mar 26, 2018 · 5 comments
Labels

Comments

@nveenjain
Copy link

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?

@fs-eire
Copy link
Contributor

fs-eire commented Mar 27, 2018

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!

✌️✌️✌️

@nveenjain
Copy link
Author

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.

@fs-eire
Copy link
Contributor

fs-eire commented Mar 28, 2018

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

@nveenjain
Copy link
Author

nveenjain commented Apr 2, 2018

@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?

@fs-eire
Copy link
Contributor

fs-eire commented Apr 4, 2018

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;
    }
};

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

No branches or pull requests

2 participants