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

0.7.0 crashes when used with bluebird's coroutines and sourcemaps #289

Closed
alubbe opened this issue Nov 30, 2015 · 16 comments
Closed

0.7.0 crashes when used with bluebird's coroutines and sourcemaps #289

alubbe opened this issue Nov 30, 2015 · 16 comments

Comments

@alubbe
Copy link

alubbe commented Nov 30, 2015

This simple test runs fine on 0.7.0 with source maps disabled and on 0.6.1 with source maps enabled, but crashes on 0.7.0 with source maps enabled:

test = require "ava"
Promise = require "bluebird"

test "peter", Promise.coroutine (t) ->
  yield Promise.delay 500

The error is

Uncaught Exception:  xxx/test2.js
  Error: No element indexed by 1
    at ArraySet_at [as at] (xxx/node_modules/source-map-support/node_modules/source-map/lib/source-map/array-set.js:83:11)
    at SourceMapConsumer_parseMappings [as _parseMappings] (xxx/node_modules/source-map-support/node_modules/source-map/lib/source-map/source-map-consumer.js:220:44)
    at SourceMapConsumer.Object.defineProperty.get (xxx/node_modules/source-map-support/node_modules/source-map/lib/source-map/source-map-consumer.js:160:14)
    at SourceMapConsumer_originalPositionFor [as originalPositionFor] (xxx/node_modules/source-map-support/node_modules/source-map/lib/source-map/source-map-consumer.js:311:43)
    at mapSourcePosition (xxx/node_modules/source-map-support/source-map-support.js:168:42)
    at wrapCallSite (xxx/node_modules/source-map-support/source-map-support.js:303:20)
    at Function.prepareStackTrace (xxx/node_modules/source-map-support/source-map-support.js:337:24)
    at Function.Promise.coroutine (xxx/node_modules/bluebird/js/release/generators.js:175:28)
    at Object.<anonymous> (xxx/test2.coffee:4:33)
    at requireFromString (xxx/node_modules/require-from-string/index.js:26:4)

With sourcemaps on, coffee produces

// Generated by CoffeeScript 1.10.0
(function() {
  var Promise, test;

  test = require("ava");

  Promise = require("bluebird");

  test("peter", Promise.coroutine(function*(t) {
    return (yield Promise.delay(500));
  }));

}).call(this);

//# sourceMappingURL=test2.js.map

and

{
  "version": 3,
  "file": "test2.js",
  "sourceRoot": "",
  "sources": [
    "test2.coffee"
  ],
  "names": [],
  "mappings": ";AAAA;AAAA,MAAA;;EAAA,IAAA,GAAO,OAAA,CAAQ,KAAR;;EACP,OAAA,GAAU,OAAA,CAAQ,UAAR;;EAEV,IAAA,CAAK,OAAL,EAAc,OAAO,CAAC,SAAR,CAAkB,UAAC,CAAD;WAC9B,OAAM,OAAO,CAAC,KAAR,CAAc,GAAd,CAAN;EAD8B,CAAlB,CAAd;AAHA"
}

Source maps and 0.7.0 work fine on synchronous tests and on those returning a promise, maybe that helps in pinpointing the issue.
I'm using [email protected] and used this command to run the tests:

coffee --compile --map . && ava test2.js
@jamestalmage
Copy link
Contributor

@alubbe

AVA supports generators out of the box. No need to wrap with Promise.coroutine.

Test:

test = require "ava"
delay = require "delay"

test "peter", (t) ->
  yield delay 500

Output:

  ✔ peter (504ms)

  1 test passed

@jamestalmage
Copy link
Contributor

To me, this looks like a problem either with coffee-scripts source-map output, or something in source-map support. Either way - it's triggered by calling bluebird.coroutine on a non generator function - which is what happens, because AVA transforms generators for you automatically.

@alubbe
Copy link
Author

alubbe commented Dec 1, 2015

How is AVA able to transform the generator when it is handled by bluebird.coroutine before ava.test? Is this a pre-runtime transformation by babel?

Leaving off the Promise.coroutine to feed AVA generators directly doesn't work for all of my tests, in particular those that use a promisifed version of request, like this:

test = require "ava"
Promise = require "bluebird"
requestAsync = Promise.promisify require "request"

test "peter", (t) ->
  yield requestAsync url: "www.google.com"

This time, the error has no stack trace:

 AssertionError: Promise rejected → Error: No element indexed by 1

If I add Promise.coroutine and run it in AVA 0.6.1, my test suite runs just fine.

@jamestalmage
Copy link
Contributor

@alubbe

Is this a pre-runtime transformation by babel?

Yes.

The following works fine (needed the http://, prefix):

test = require "ava"
Promise = require "bluebird"
requestAsync = Promise.promisify require "request"

test "peter", (t) ->
  yield requestAsync url: "http://www.google.com"

Turning sourcemaps off gave a cleaner error message with source-maps off.

Might be related:
webpack/webpack#1071

Also, googling Error: No element indexed by 1, brings up a ton of issues - all having to do with source-maps.

@novemberborn - Do you have any insights here?

@vadimdemedes
Copy link
Contributor

All code examples in this thread are missing *, which actually marks a generator function, otherwise it's not.

test('something', function * (t) {
  yield requestAsync(...);
});

@alubbe let me know if that works!

@jamestalmage
Copy link
Contributor

@vdemedes - These are all coffee-script examples. You don't use * in coffee-script, generator functions are implied by use of the yield keyword.

@vadimdemedes
Copy link
Contributor

@jamestalmage oh, what a mess...

@jamestalmage
Copy link
Contributor

Also, in these the Error: No element indexed by 1 error is always masking another error. In the second example it was improper use of the request API. The problem is that, instead of exposing the actual underlying error, source-map-support is choking on the provided source-maps when it tries to beautify the thrown error and throwing a different one entirely.

This issue (choking on source-maps, and masking error messages that might be helpful), are really what bothers me. At a minimum source-map-support should append the original stack trace to it's own if it fails to transform it.

@jamestalmage
Copy link
Contributor

@novemberborn
Copy link
Member

@jamestalmage that PR seems like a good step to at least avoid not receiving any stack trace.

I'll try and reproduce the issue, see if I can step through the source-map code to find out more.

@novemberborn
Copy link
Member

The compiled coffee-script code means that there is no async/await for AVA to compile. It just sees a test function which returns a promise. So that's not the issue.

0.7.0 contains #273 which passes an input source map to the Babel compiler used by AVA. If I disable the input source map I see this source map for Babel's output:

{ version: 3,
  sources: [ '/private/var/folders/_6/p8qxp_3n62zg9081tvb0lcc80000gn/T/tmp.TAYPmrUt/test2.js' ],
  names: [],
  mappings: ';;;AACA,CAAC,YAAW;AACV,MAAI,OAAO,EAAE,IAAI,CAAC;;AAElB,MAAI,GAAG,OAAO,CAAC,KAAK,CAAC,CAAC;;AAEtB,SAAO,GAAG,OAAO,CAAC,UAAU,CAAC,CAAC;;AAE9B,MAAI,CAAC,OAAO,EAAE,OAAO,CAAC,SAAS,CAAC,WAAU,CAAC,EAAE;AAC3C,WAAQ,MAAM,OAAO,CAAC,KAAK,CAAC,GAAG,CAAC,CAAE;GACnC,CAAC,CAAC,CAAC;CAEL,CAAA,CAAE,IAAI,WAAM,CAAC',
  file: '/private/var/folders/_6/p8qxp_3n62zg9081tvb0lcc80000gn/T/tmp.TAYPmrUt/test2.js',
  sourcesContent: [ '// Generated by CoffeeScript 1.10.0\n(function() {\n  var Promise, test;\n\n  test = require("ava");\n\n  Promise = require("bluebird");\n\n  test("peter", Promise.coroutine(function*(t) {\n    return (yield Promise.delay(500));\n  }));\n\n}).call(this);\n\n//# sourceMappingURL=test2.js.map\n' ] }

With the input source map I get:

{ version: 3,
  sources: [ 'test2.coffee' ],
  names: [],
  mappings: ';;;AAAA,CAAA,YAAA;AAAA,MAAA,OAAA,EAAA,IAAA,CAAA;;AAAA,MAAA,GAAO,OAAA,CAAQ,KAAR,CAAA,CAAA;;AACP,SAAA,GAAU,OAAA,CAAQ,UAAR,CAAA,CAAA;;AAEV,MAAA,CAAK,OAAL,EAAc,OAAO,CAAC,SAAR,CAAkB,WAAC,CAAD,EAAA;ACM5B,WDLF,MAAM,OAAO,CAAC,KAAR,CAAc,GAAd,CAAN,CAAA;GADY,CAAd,CAAA,CAAA;CCSC,CAAA,CAAE,IAAI,WAAM,CAAC',
  file: 'test2.js',
  sourcesContent:
   [ null,
     '// Generated by CoffeeScript 1.10.0\n(function() {\n  var Promise, test;\n\n  test = require("ava");\n\n  Promise = require("bluebird");\n\n  test("peter", Promise.coroutine(function*(t) {\n    return (yield Promise.delay(500));\n  }));\n\n}).call(this);\n\n//# sourceMappingURL=test2.js.map\n' ] }

Note that sources now points at test2.coffee rather than test2.js and that sourcesContent now has two entries, the first being null.

Turns out Bluebird throws an error when it's async module is loaded. This then causes a crash from source-map-support.

This seems like a bug in Babel's handling of input source maps. Maybe it's already fixed in Babel 6.

@jamestalmage
Copy link
Contributor

The compiled coffee-script code means that there is no async/await for AVA to compile. It just sees a test function which returns a promise. So that's not the issue.

This is a generator/yield example, and coffee-script outputs an actual generator function, (i.e. function *() {...}). async/await support is not in coffee-script yet. (I take your point though - the problem is a bad source map).

Turns out Bluebird throws an error when it's async module is loaded. This then causes a crash from source-map-support.

That is wrapped in a try/catch. Any crash of source-map-support would be caught.

Note that sources now points at test2.coffee rather than test2.js and that sourcesContent now has two entries, the first being null.

That certainly seems like a bug. Can you submit an issue on the Babel tracker? (or hunt down an existing one and link it here).

Maybe it's already fixed in Babel 6.

Hopefully. The issue blocking our move to Babel 6 appears to finally be getting some traction. There is even a PR to fix it.

@novemberborn
Copy link
Member

This is a generator/yield example, and coffee-script outputs an actual generator function, (i.e. function *() {...}).

It outputs test("peter", Promise.coroutine(function*(t) { actually. I haven't checked but I assume that returns a function which returns a promise. The test passes if input source maps are disabled.

That is wrapped in a try/catch. Any crash of source-map-support would be caught.

Ah yes that's the call to prepareStackTrace. The error is stored in Async.firstLineError which is used in the final build for debugging purpose (see build tool line). And that's when it crashes.

Can you submit an issue on the Babel tracker? (or hunt down an existing one and link it here).

I'm thoroughly confused regarding their code base and newfangled issue tracker, so that may be tricky. Even if it's not yet fixed in Babel 6 I doubt they'd backport any fixes?

I'll see if I can reproduce the transformation with Babel 6 instead. That'll be a good start.

@jamestalmage
Copy link
Contributor

I'm thoroughly confused regarding their code base and newfangled issue tracker,

Why do you think I punted this to you? :trollface:

Seriously though, the issue tracker is pretty straightforward once you dig in. As for the codebase, I feel your pain. I would recommend browsing a clean clone (don't run tests in the clean clone - it generates lots of additional sources that are just confusing until you get a basic grasp of the layout and build system).

@novemberborn
Copy link
Member

Transforming test2.js with [email protected] gives an entirely different source map:

{ version: 3,
  file: 'test2.js',
  sourceRoot: '',
  sources: [ 'test2.coffee' ],
  names: [],
  mappings: ';AAAA,aAAA;MAAA;;AAAA,MAAA,GAAO,OAAA,CAAQ,KAAR;;AACP,SAAA,GAAU,OAAA,CAAQ,UAAR;;AAEV,MAAA,CAAK,OAAL,EAAc,OAAO,CAAC,SAAR,CAAkB,WAAC,CAAD;AAC9B,iBAAM,OAAO,CAAC,KAAR,CAAc,GAAd,CAAN,CAD8B;GAAlB,CAAd,EAHA' }

If I force that into lib/babel the test doesn't crash.

I found babel/babel@0ef6072 which links to babel/babel#2522 and babel/babel-loader#116 which discuss a similar "No element indexed by 1" error.

Indeed when I inject a source map generated using [email protected] (the last release before that commit) the error reoccurs. In conclusion it looks like Babel < 6.1.2 does not correctly merge input source maps.

Upgrading to Babel 6 should solve this issue. I suppose we could consider disabling input source maps but IMO that's not worth the trouble.

(Can't find a corresponding issue in the Babel bugtracker though 😉)

@alubbe
Copy link
Author

alubbe commented Jan 1, 2016

Good news, 0.9.x fixes this issue due to the new babel version!

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

No branches or pull requests

4 participants