panic with arrayStash about otto HOT 10 CLOSED

I am able to reproduce this fairly reliably. Here is the JavaScript we are running.

I'm pretty sure that this JS can run just fine under some conditions. But when we have a branch where doc.channel_id == doc._id we get the crash. Maybe my monkey-patching Array is part of it. Or maybe calling .concat() or indexOf(). Those are the things that stand out about the crashing code path.

function(doc, oldDoc, userCtx, secObj) {
  if(!Array.isArray) {
    Array.isArray = function (vArg) {
      return Object.prototype.toString.call(vArg) === "[object Array]";
    };
  }
  if (doc.channel_id) {
    // doc belongs to a channel
    channel("ch-"+doc.channel_id);
    // this document describes a channel
    if (doc.channel_id == doc._id) {
      // magic document, treat it carefully
      if (oldDoc && oldDoc.owners.indexOf(userCtx.name) == -1) {
        throw({unauthorized:"you are not a channel owner"});
      }
      if (!Array.isArray(doc.owners)) {
        throw({forbidden : "owners must be an array"})
      }
      // grants access to the channel to all members and owners
      var members = [];
      if (Array.isArray(doc.members)) {
        members = doc.members;
      }
      var them = doc.owners.concat(members);
      access(them, "ch-"+doc._id);

      // notify members of the new channel
      // TODO this can be removed once
      // https://github.com/couchbaselabs/sync_gateway/issues/31
      // is fixed
      for (var i = 0; i < them.length; i++) {
        channel("chs-"+them[i]);
      };

    }
  }
  if (doc.type == "profile") {
    channel("profiles");
    var user = doc._id.substring(doc._id.indexOf(":")+1);
    if (userCtx.name != user) {
      throw({unauthorized : "you can only edit your own profile"});
    }
    access(user, "profiles");
    // TODO remove when #31 is fixed
    access(user, "chs-"+user);
  }
}

from otto.

Robert — this might be a regression from 149f94f?

from otto.

Hi, thanks for the report. The problem is that the array code is pretty stale. I was actually in the middle of reworking it to generalize it when you had your issue.

Technical detail: Some bits of code always assume an _arrayStash if the [[Class]] of the object is "Array", which is incorrect. Array.concat is one example of this.

from otto.

I was able to reproduce it with a very minimal function:

function(doc) {doc.x.concat(doc.y);}

where the doc argument is {"x":["abc"],"y":["xyz"]}

Interestingly, if I use array literals within the source code there is no panic:

function(doc) {["xyz"].concat(["abc"]);}

from otto.

I tried to patch TestArray_concat to do this, but I can't figure out how to pass the result of

json.Unmarshal([]byte(`[0,1,2]`), []int)

in as an argument to be used as abc.

Here is the test I used in Sync Gateway to narrow down our crashing function to the minimal case.

from otto.

The property accessor/stash system has been undergoing a total rewrite in order to be more robust and compliant (to the ECMA specification). Hopefully it will be released by Monday. I'll take a look at these tests and see how to get them passing.

from otto.

I've added a branch (issue16) with the rewrite, which is looking pretty good as far as I can tell:

https://github.com/robertkrimen/otto/tree/issue16

Here is a test which I think addresses the problem you were having (which is now passing):

https://github.com/robertkrimen/otto/blob/issue16/bug_test.go#L84

from otto.

This is now in master.

There is a slight compliance issue with Array.concat treating a "GoArray" as an "Array". Technically, a "GoArray" is not an "Array", but I think that making .concat behave as if it was is DWIM.

from otto.

I updated sync_gateway to the latest otto, but my test is still failing:

https://github.com/couchbaselabs/sync_gateway/blob/master/src/github.com/couchbaselabs/sync_gateway/channels/channelmapper_test.go#L29

(to get the environment right you pretty much have to check out sync gateway with all it's submodules, and run it with ./test.sh from the repository root...)

Will update if I get any more hints.

from otto.

But it's not panicking right? It's not panicking here.

I got your tests passing when I addressed the issue of changing the type_go_array .Class() from "Array" to "GoArray"

value.Export() needs to be updated on this end.

from otto.