att / rcloud

Collaborative data analysis and visualization
http://rcloud.social
MIT License
431 stars 142 forks source link

Rserve.wrap_ocap should detect errors #669

Open gordonwoodhull opened 10 years ago

gordonwoodhull commented 10 years ago

It passes the error forward as a valid result, where I think it should use the error continuation instead:

        s.OCcall(ocap, values, function(err, v) {
            if (!_.isUndefined(v))
                v = Rserve.wrap_all_ocaps(s, v); 
            k(err, v);
        });

If v contains an error, the continuation is still called with err==null and v containing the error.

The particular error I've got at the moment would be easy to detect:

> _.extend({}, v)
    Object {0: "Error in fun(...) : could not find function "RC.get.range"↵", r_type: "string_array", r_attributes: Object}
        0: "Error in fun(...) : could not find function "RC.get.range"↵"
      r_attributes: Object
        class: "try-error"
        condition: Object
        __proto__: Object
        r_type: "string_array"
        __proto__: Object

.. and I expect that most exceptions from an R ocap will have a similar format

cscheid commented 10 years ago

Yeah, I think this is really a bug, actually.

cscheid commented 10 years ago

I can fix that if you'd rather do something else.

gordonwoodhull commented 10 years ago

Thanks.. I'm working on dc, actually... I just report bugs as I find them.

Ran into this while trying to figure out whether leaflet notebooks still work with the AMD stuff. I think they do but the particular notebook I was testing needed a library and some data which I don't have locally.

cscheid commented 10 years ago

I would claim this is a bug in rserve-js rather than rcloud, but in the rserve-js test suite there is a test specifically about raising exceptions and making sure they're handled by the error path (it's a bluebird catch). So I'm confused now.

EDIT: Yeah, there's definitely something bizarre happening. I'm going to have to confer with Simon on Friday to see what's up.

gordonwoodhull commented 10 years ago

Related:

gordonwoodhull commented 7 years ago

@s-u, I believe this is the issue we were talking about. Almost three years old, wow.

Pretty intense systems hacking. I would love to work on this if I can find a few days of concentration.

Like #1376, I think this may have unexpected consequences since there may be errors being thrown and ignored, accidentally or harmlessly, which actually should be ignored. So I'm going to be cautious and put this into 1.8 (which we can deploy to research early if we like).

gordonwoodhull commented 5 years ago

2.2 for reals