Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expected int32 as second argument in Firefox with Array.prototype.map #347

Closed
elicwhite opened this issue Jun 12, 2015 · 22 comments
Closed

Comments

@elicwhite
Copy link

When I'm running my test suite in Firefox, I get this error. I haven't been able to successfully track down where my code is calling map such that this would fail which has been quite frustrating. Have you guys seen this before or know what it might be? This error does not occur on Chrome or Safari.

Firefox 38.0.0 (Mac OS X 10.10) ERROR
  Error: Expected int32 as second argument
  at 
node_modules/es6-shim/es6-shim.js:928:0

If I remove the part of the shim for Array.prototype.map by commenting out:

if (!toLengthsCorrectly(Array.prototype.map)) {
    var originalMap = Array.prototype.map;
    overrideNative(Array.prototype, 'map', function map(callbackFn) {
      return _apply(originalMap, this.length >= 0 ? this : [], arguments);
    }, true);
  }

then my test suite runs perfectly in Firefox. Any ideas?

@ljharb
Copy link
Collaborator

ljharb commented Jun 12, 2015

I have v38.0.1 on my Mac and I don't get this error. Which version of the es6-shim are you using?

Also, presumably this isn't happening when the shim is loaded, but when your code runs later - how are you calling .map in your own code?

@ljharb
Copy link
Collaborator

ljharb commented Jun 12, 2015

Note that this error is actually what happens in an unshimmed firefox if you do Array.prototype.map.call({ 0: 1, length: -1}, function () {}) since it's improperly handling negative lengths.

Are you using Array.prototype.map with a non-array anywhere?

ljharb added a commit that referenced this issue Jun 12, 2015
This fails on `Array#map` with a clearer message in Firefox v38. Relates to #347.
@elicwhite
Copy link
Author

Hmm, I'll look closer at our codebase. I'm unsure why the shim itself is causing this problem. When I remove that section from the shim, our tests all pass in Firefox. That seems like it would imply it being an issue in the shim, right?

@ljharb
Copy link
Collaborator

ljharb commented Jun 17, 2015

It definitely means it's something in the shim conflicting with something in your codebase. However, you could probably remove the other conflict and fix it too :-)

Are you using anything that might override Array.prototype.map? Are you using the es5-shim, but loading it after es6-shim?

@elicwhite
Copy link
Author

I'm still trying to track this down.

I changed the map part of the shim to:

if (!toLengthsCorrectly(Array.prototype.map)) {
    var originalMap = Array.prototype.map;
    overrideNative(Array.prototype, 'map', function map(callbackFn) {

      try {
        return _apply(originalMap, this.length >= 0 ? this : [], arguments);
      }
      catch (e) {
        console.error('failed', originalMap, this.length >= 0 ? this : []);
        throw e;
      }

    }, true);
  }

as that should give us the information we need about what might be failing.

Here is a snippit from my output:

Firefox 38.0.0 (Mac OS X 10.10.0) ERROR: 'failed', function map() { ... }, ['lala']
..
Firefox 38.0.0 (Mac OS X 10.10.0) [REDACTED] FAILED
  Expected int32 as second argument
  map@/var/folders/mg/64k6_7p16vn2jwbtlz41_f6w0000gp/T/6897b5f55636a5d20259f7caa3712929.browserify:16243:16 <- ../node_modules/es6-shim/es6-shim.js:931:0
  d3_selection_classed@/var/folders/mg/64k6_7p16vn2jwbtlz41_f6w0000gp/T/6897b5f55636a5d20259f7caa3712929.browserify:6514:12 <- ../node_modules/d3/d3.js:707:0
  require<[37]</</d3_selectionPrototype.classed@/var/folders/mg/64k6_7p16vn2jwbtlz41_f6w0000gp/T/6897b5f55636a5d20259f7caa3712929.browserify:6505:22 <- ../node_modules/d3/d3.js:698:0
  ...

Firefox 38.0.0 (Mac OS X 10.10.0) ERROR: 'failed', function map() { ... }, ['lala']
Firefox 38.0.0 (Mac OS X 10.10.0) [REDACTED] FAILED
  Expected int32 as second argument
  map@/var/folders/mg/64k6_7p16vn2jwbtlz41_f6w0000gp/T/6897b5f55636a5d20259f7caa3712929.browserify:16243:16 <- ../node_modules/es6-shim/es6-shim.js:931:0
  d3_selection_classed@/var/folders/mg/64k6_7p16vn2jwbtlz41_f6w0000gp/T/6897b5f55636a5d20259f7caa3712929.browserify:6514:12 <- ../node_modules/d3/d3.js:707:0
  require<[37]</</d3_selectionPrototype.classed@/var/folders/mg/64k6_7p16vn2jwbtlz41_f6w0000gp/T/6897b5f55636a5d20259f7caa3712929.browserify:6505:22 <- ../node_modules/d3/d3.js:698:0
[REDACTED].js

And here is a larger snippit with more examples (not just coming from d3). http://pastebin.com/PnK5xbev

Any thoughts? The inputs look reasonable relative to other calls that don't fail. I haven't been able to get to a individual failing test case though. When I run just the tests that fail, they pass. Still digging in.

FWIW, this is still Firefox only.

@ljharb
Copy link
Collaborator

ljharb commented Jul 19, 2015

In your console.error path, can you include the arguments also? That's the part triggering the error :-)

@classiemilio
Copy link

I was able to narrow it down further. All my tests pass when I don't call toLengthsCorrectly(Array.prototype.map).

 var toLengthsCorrectly = function (method, reversed) {
    var obj = { length: -1 };
    obj[reversed ? ((-1 >>> 0) - 1) : 0] = true;
    return valueOrFalseIfThrows(function () {
      _call(method, obj, function () {
        // note: in nonconforming browsers, this will be called
        // -1 >>> 0 times, which is 4294967295, so the throw matters.
        throw new RangeError('should not reach here');
      }, []);
    });
  };

That looks in line with what you're saying about an object having a length of -1.

Also, the RangeError is called in Firefox.

@ljharb
Copy link
Collaborator

ljharb commented Sep 2, 2015

fwiw, the latest Firefox Nightly made a change to Array#map that actually avoids this error from being thrown - so I suspect that once that lands in stable, this problem may go away. I've still been able to reproduce it, and from our offline discussion it seems only reproducible with karma + firefox :-/

@elicwhite
Copy link
Author

I look forward to not needing our hack in front of the polyfill. :)

@mikeazo
Copy link

mikeazo commented Oct 1, 2015

@ljharb Do you have a link to the Firefox changes? I'd like to track to see when it makes it into stable.

@ljharb
Copy link
Collaborator

ljharb commented Oct 1, 2015

https://bugzilla.mozilla.org/show_bug.cgi?id=924058 is the overall ToLength bug, but https://bugzilla.mozilla.org/show_bug.cgi?id=1200108 is specific to "map", and the specific one that fixed this issue.

@timglaser
Copy link

Is anyone aware of an existing fork of es6-shim that is modified to not cause these errors in Firefox 42 and prior? Ideally it would be a fork that is to be kept up to date with the latest of es6-shim but with the Firefox fix in place until Firefox 45 is released for those that follow the convention of supporting the latest two versions of browsers. So far for my needs are met by commenting out the if statement around the Array.prototype.map wrapping at https://github.com/paulmillr/es6-shim/blob/master/es6-shim.js#L1244. But I'd rather not include have to maintain a dependency in my own repo.

@ljharb
Copy link
Collaborator

ljharb commented Nov 18, 2015

@timglaser this specific error was only occurring with the combination of Karma and Firefox. Is it happening for you in FF 42 by itself? If so, I will absolutely fix it.

@timglaser
Copy link

@ljharb it is. I'm getting the "expected Int32 second argument" error in production where karma is not included in any form. I just did a double check to make sure I'm not lying :) I'm getting the error in FF 42, but not in FF43, and not in FF 42 if I apply the change I mentioned above (and so far I'm not finding any negative effects in any of our supported browsers due to the change).

@ljharb
Copy link
Collaborator

ljharb commented Nov 18, 2015

@timglaser on what OS? I can't reproduce it. Can you make a jsfiddle that repros it?

@timglaser
Copy link

@ljharb This is happening on OS X and Windows versions of Firefox 42 and Firefox ESR. I'll see if I can get a jsfiddle together. Not likely today. It could be that something in our package other than karma is interacting in a way that highlights this bug. Thanks for being responsive on this!

@ljharb
Copy link
Collaborator

ljharb commented Nov 18, 2015

@timglaser Absolutely - I will happily fix anything I can reproduce. The remainder of this issue is because with karma, try/catch simply wasn't working - which creates a scenario that's unfixable. Hopefully your problem isn't equally unfixable :-)

@ljharb
Copy link
Collaborator

ljharb commented Dec 15, 2015

@TheSavior @timglaser can you check this again as of c7ff222? I fixed a number of issues regarding toLengthsCorrectly in older Firefoxes, and I'm hoping it's fixed this issue.

@cholewczuk
Copy link

I had the same problem on prod with FF v29 - v42. I can confirm that c7ff222 helped. Thanks!

@ljharb
Copy link
Collaborator

ljharb commented Dec 15, 2015

Awesome! It's released as v0.34.0 so hopefully that will resolve this issue.

@elicwhite
Copy link
Author

0.34.0 fixed it for us as well.

@ljharb
Copy link
Collaborator

ljharb commented Dec 16, 2015

Hooray, glad I finally found a solution, and was able to reproduce it without Karma!

@ljharb ljharb self-assigned this Dec 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants