Test for array failing about avsc HOT 8 CLOSED

Thanks for reporting this. Could you tell me more about how the error occurs? Are you using iframes and passing the schema from one to another?

from avsc.

HI,

I am trying to wrap this into a Node-RED node (http://nodered.org)
To do that I'm trying to pass in the schema as a message so it is being handled in a callback

this.on("input", function(msg) {
            if (msg.hasOwnProperty("schema")) {
                try { this.s = avro.parse(msg.schema); }
                catch(e) { node.log(e.toString()); node.error("Invalid schema def",msg.schema); }
            }

This always fails even with a valid schema - it is fine handled outside of the callback.

from avsc.

I see. I'm not familiar with node-red, but my suspicion is that each node gets run in a separate context (similarly to an iframe) which causes this error when objects are shared between nodes.

I also did some preliminary benchmarking and switching to Array.isArray looks acceptable. However there are other instanceof checks which aren't as easily replaced, so this will require a bit more thought to make sure everything works as expected.

Do you think you'd be able to extract a small test case or script to reproduce this failure?

from avsc.

Yes - I did see lots of other instanceof in there - though quite a few seemed to be to do with ****Type properties which being defined by you should all be ok... so maybe just the other Array ones need to be looked at. Certainly (so far with the simple schema I have used) just those two lines seemed to fix it - but I guess that's naive. The lines I spotted were 134, 678, 724, 966, 1158, 1219, 1250, 1297, 1358, 1400, and 1473..

In meantime I'll go try to create simple test case.

from avsc.

Matthew, you're right that a problem with instanceof is that objects created in a different context (window, iframe. etc.) don't share the same prototype chain.

btw, here's an implementation that works even if Array.isArray isn't defined:
https://github.com/yields/isArray/blob/master/index.js

This is consistent with the Polyfill definition here:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/isArray

-Steve

from avsc.

Simple test example

var avro = require('avsc');
var vm = require("vm");

var schema = '{ "name": "Pet", "type": "record", "fields": [ {"name": "kind", "type": {"name": "Kind", "type": "enum", "symbols": ["CAT", "DOG"]}}, {"name": "name", "type": "string"} ]}';

var func = 'var schema = JSON.parse(schema);\
    console.log(schema);\
    var type = avro.parse(JSON.parse(s));\
    var buf = type.toBuffer({"kind":"CAT", "name":"Albert"});\
    console.log(buf);\
    var val = type.fromBuffer(buf);\
    console.log(val);';

var sandbox = {
    avro:avro,
    console:console,
    schema:schema
}
var context = vm.createContext(sandbox);
var script = vm.createScript(func);
script.runInContext(context);
console.log(context.results);

from avsc.

Thanks for the example @dceejay! I adapted it and was able to reproduce the failure.

Rather than only replace array instanceof checks, I went ahead and replaced all uses of instanceof with context-agnostic checks (this should prevent surprises if sharing types across node-red nodes for example). Version 3.4.0 is available with those changes.

@stevelr - thanks for the extra information.

from avsc.

confirmed working here - thanks for the fix(es).

from avsc.