Discord fails when encountering some preprocessor imports

[groovecoder]

We enabled the hook on the kuma code-base, and merged mdn/kuma@d575949 into master.

They payload delivered successfully to the hook, but the heroku logs show ...

2015-06-10T19:46:07.348792+00:00 heroku[router]: at=info method=POST path="/hook" host=youshoulduse.herokuapp.com request_id=5767898a-3792-4337-80cb-3d52c0539b24 fwd="192.30.252.45" dyno=web.1 connect=1ms service=42ms status=200 bytes=211
2015-06-10T19:46:07.332623+00:00 app[web.1]: Wed, 10 Jun 2015 19:46:07 GMT express deprecated res.send(status, body): Use res.status(status).send(body) instead at module.js:13:9
2015-06-10T19:46:07.342132+00:00 app[web.1]: Repo: mozilla/kuma
2015-06-10T19:46:08.603048+00:00 app[web.1]:     return sourceMap.sections != null
2015-06-10T19:46:08.603050+00:00 app[web.1]:                     ^
2015-06-10T19:46:08.603052+00:00 app[web.1]: TypeError: Cannot read property 'sections' of undefined
2015-06-10T19:46:08.603042+00:00 app[web.1]: /app/node_modules/source-map/lib/source-map/source-map-consumer.js:23
2015-06-10T19:46:08.603055+00:00 app[web.1]:     at /app/sourceParse.js:22:26
2015-06-10T19:46:08.603057+00:00 app[web.1]:     at Renderer.render (/app/node_modules/stylus/lib/renderer.js:107:12)
2015-06-10T19:46:08.603054+00:00 app[web.1]:     at new SourceMapConsumer (/app/node_modules/source-map/lib/source-map/source-map-consumer.js:23:21)
2015-06-10T19:46:08.603061+00:00 app[web.1]:     at Request.emit (events.js:110:17)
2015-06-10T19:46:08.603060+00:00 app[web.1]:     at Request.self.callback (/app/node_modules/request/request.js:354:22)
2015-06-10T19:46:08.603058+00:00 app[web.1]:     at Request._callback (/app/sourceParse.js:21:17)
2015-06-10T19:46:08.603063+00:00 app[web.1]:     at Request.<anonymous> (/app/node_modules/request/request.js:1207:14)
2015-06-10T19:46:08.603064+00:00 app[web.1]:     at Request.emit (events.js:129:20)
2015-06-10T19:46:08.603067+00:00 app[web.1]:     at IncomingMessage.emit (events.js:129:20)
2015-06-10T19:46:08.603066+00:00 app[web.1]:     at IncomingMessage.<anonymous> (/app/node_modules/request/request.js:1153:12)

[Faeranne]

A bit of debugging seems to suggest that the Stylus engine is choking on the styl file causing the sourcemap to not be built. Since the callbacks are eating the stylus errors, I'm not 100% certain, but I bet adding an error callback to the stylus call should clear that up.

[darkwing]

I inadvertantly triggered this same error with a very basic PR:

https://github.com/darkwing/YouShouldUseTest/pull/7/files

In that example, a syntax error is presented (@include instead of @import);

[openjck]

I wonder... does it fail on all @includes?

[Faeranne]

So how should we handle this? Do we alert the user through a comment? I don't want to derail the main goal of this project.

[darkwing]

Good question @mrmakeit . I was thinking the same -- I don't think there's much we can do outside of letting the user know their stylus cannot be parsed. I think that is very reasonable. @groovecoder @openjck ?

[groovecoder]

👍 - that's a valuable comment in its own way.

[openjck]

I think a comment on the pull request (a discussion comment, not a line comment) explaining that the CSS can't be tested would be great! 😄

[openjck]

I opened #67 for commenting when stylesheets cannot be tested.

But that may not be what caused this issue. Does Stylus testing fail whenever @import is used? Let's keep this issue open to continue to analyze the error Luke posted.

[darkwing]

I get the feeling that any type of @import is going to be a problem where we may eventually need to actually clone the repo and evaluate its entire stylus stack. We also need to be aware of their stylus version.

A very short solution could be to ignore calls to @import, but we'll likely end up getting errors if said imports contain important mixins.

[Faeranne]

We could do some preprocessing, and directly include the file before sending it through the processor. I'm not sure if there would be another way beyond what @darkwing suggested, and I doubt that would be great for performance, especially on larger repos.