core.async

0.1.346, using >! crashes with no method 'setTimeout', when targeting node

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Environment:
    [[org.clojure/clojure "1.6.0"]
     [org.clojure/core.async "0.1.346.0-17112a-alpha"]
     [org.clojure/clojurescript "0.0-2511"]]

Description

When targeting node, running a simple >! crashes immediately:

(let [a (chan)]
  (go (>! a :hi))
  (go (println (<! a))))

Crashes with:

goog.global.setTimeout(cb, 0);
   ^
TypeError: Object #<Object> has no method 'setTimeout'

This works fine with 0.1.338.0-5c5012-alpha.

Activity

Hide
Joel Wilsson added a comment -

The problem is really in the ClojureScript compiler. It does not export those NodeJS global functions. I have a fix that works for me and seems sound, so I submitted a pull request: https://github.com/clojure/clojurescript/pull/45

Show
Joel Wilsson added a comment - The problem is really in the ClojureScript compiler. It does not export those NodeJS global functions. I have a fix that works for me and seems sound, so I submitted a pull request: https://github.com/clojure/clojurescript/pull/45
Hide
David Nolen added a comment -

I'm not convinced yet that there are any issues with the ClojureScript compiler nor that your PR represents the solution we should pursue.

Before we even begin assessing any solution first someone needs to identify precisely why the goog.async.nextTick detection doesn't pick up the Node.js globals. The first thing that Google Closure library does is map the global environment to goog.global. I'm curious as to why this isn't working in this case.

Show
David Nolen added a comment - I'm not convinced yet that there are any issues with the ClojureScript compiler nor that your PR represents the solution we should pursue. Before we even begin assessing any solution first someone needs to identify precisely why the goog.async.nextTick detection doesn't pick up the Node.js globals. The first thing that Google Closure library does is map the global environment to goog.global. I'm curious as to why this isn't working in this case.
Hide
Joel Wilsson added a comment -

Patch from the pull request attached.

Show
Joel Wilsson added a comment - Patch from the pull request attached.
Hide
Joel Wilsson added a comment -

Yes, the first thing the Google Closure library is set up a reference to the global environment with goog.global = this;

However, this is not the global environment in Node.js. From http://nodejs.org/api/vm.html#vm_sandboxes:

the this expression within the global scope of the context evaluates to the empty object ({}) instead of to your sandbox.

If you start a Node.js REPL and enter console.log(this) you will see that this is in fact the global environment. However, if you put that statement in a file and run it with nodejs <filename>, the output will be "{}".

Some possible solutions:

  1. Use goog.exportSymbol to get these functions into goog.global, as in the patch I attached. A drawback of this approach is that it is using goog to fix goog, so it must be done after goog has been loaded.
  2. Explicitly set properties on this before doing anything else. This is the simplest solution. Preamble:
    this.setTimeout = setTimeout;
    this.clearTimeout = clearTimeout;
    this.setInterval = setInterval;
    this.clearInterval = clearInterval;
  3. Use vm.Script.runInThisContext, as bootstrap_node.js does to load base.js with this referencing the global environment found in the variable global.
    A bit tricky in this case, since the file we're executing needs to read itself, but it can be done with this preamble:
    if (!this.__nodeFilename) {
        var vm = require('vm');
        var fs = require('fs');
        global.__nodeFilename = __filename;
        vm.runInThisContext.call(global, fs.readFileSync(__nodeFilename), __nodeFilename);
        process.exit(0);
    }

    For this to work the hashbang must be removed, since #!/usr/bin/env node is not valid JavaScript. Since node is a symlink to ax25-node on Ubuntu, and the Node.js binary is called "nodejs", this might be a good idea either way.
    While this solution is the closest to bootstrap_node.js, it is complex and may be a bit too clever.

  4. Convince the Closure library maintainers to check if base.js is being loaded in Node.js and do goog.global = global; if that is the case.

With 3 and 4, goog.global will contain a lot of Node.js stuff. That may or may not be a good thing. Considering the effort in bootstrap node to hide module and exports, I'm guessing it's a bad thing.

From what I can tell, programs using the Closure library, and which have been compiled by the Closure compiler into a single file, have never worked properly if they have been using anything that needs setTimeout and friends.

Show
Joel Wilsson added a comment - Yes, the first thing the Google Closure library is set up a reference to the global environment with goog.global = this; However, this is not the global environment in Node.js. From http://nodejs.org/api/vm.html#vm_sandboxes:
the this expression within the global scope of the context evaluates to the empty object ({}) instead of to your sandbox.
If you start a Node.js REPL and enter console.log(this) you will see that this is in fact the global environment. However, if you put that statement in a file and run it with nodejs <filename>, the output will be "{}". Some possible solutions:
  1. Use goog.exportSymbol to get these functions into goog.global, as in the patch I attached. A drawback of this approach is that it is using goog to fix goog, so it must be done after goog has been loaded.
  2. Explicitly set properties on this before doing anything else. This is the simplest solution. Preamble:
    this.setTimeout = setTimeout;
    this.clearTimeout = clearTimeout;
    this.setInterval = setInterval;
    this.clearInterval = clearInterval;
  3. Use vm.Script.runInThisContext, as bootstrap_node.js does to load base.js with this referencing the global environment found in the variable global. A bit tricky in this case, since the file we're executing needs to read itself, but it can be done with this preamble:
    if (!this.__nodeFilename) {
        var vm = require('vm');
        var fs = require('fs');
        global.__nodeFilename = __filename;
        vm.runInThisContext.call(global, fs.readFileSync(__nodeFilename), __nodeFilename);
        process.exit(0);
    }
    For this to work the hashbang must be removed, since #!/usr/bin/env node is not valid JavaScript. Since node is a symlink to ax25-node on Ubuntu, and the Node.js binary is called "nodejs", this might be a good idea either way. While this solution is the closest to bootstrap_node.js, it is complex and may be a bit too clever.
  4. Convince the Closure library maintainers to check if base.js is being loaded in Node.js and do goog.global = global; if that is the case.
With 3 and 4, goog.global will contain a lot of Node.js stuff. That may or may not be a good thing. Considering the effort in bootstrap node to hide module and exports, I'm guessing it's a bad thing. From what I can tell, programs using the Closure library, and which have been compiled by the Closure compiler into a single file, have never worked properly if they have been using anything that needs setTimeout and friends.
Hide
David Nolen added a comment -

Joel, many thanks for all the extra information. Will sort through this and think about it.

Show
David Nolen added a comment - Joel, many thanks for all the extra information. Will sort through this and think about it.
Hide
David Nolen added a comment -

Issue is confirmed on this end. Will look into.

Show
David Nolen added a comment - Issue is confirmed on this end. Will look into.
Hide
David Nolen added a comment -

So I looked into this a bit, this does not actually appear to be a real issue, at least not yet. The Node.js environment must be bootstrapped for ClojureScript to work. However this process could be simpler and we will address the usability bits in ClojureScript. When it's resolved there and we can confirm via automated testing in core.async we will close this ticket.

Show
David Nolen added a comment - So I looked into this a bit, this does not actually appear to be a real issue, at least not yet. The Node.js environment must be bootstrapped for ClojureScript to work. However this process could be simpler and we will address the usability bits in ClojureScript. When it's resolved there and we can confirm via automated testing in core.async we will close this ticket.
Hide
Stuart Mitchell added a comment -

Not quite sure what you mean by 'not a real problem' as I have a clojurescript project that targets atom-shell that worked with 0.1.338.0-5c5012-alpha but now doesn't.

Do you mean that its actually a clojurescript issue, if so is there a bug report/ work around?

Show
Stuart Mitchell added a comment - Not quite sure what you mean by 'not a real problem' as I have a clojurescript project that targets atom-shell that worked with 0.1.338.0-5c5012-alpha but now doesn't. Do you mean that its actually a clojurescript issue, if so is there a bug report/ work around?
Hide
Joel Wilsson added a comment -

Stuart, could you please try the four lines from option 2 in my previous comment as a preamble and let me know if that solves the problem for you? Ie. put them in a file preamble.js on your project's classpath (I use resources/) and add :preamble "preamble.js" to your :compiler settings. Assuming that you are using lein-cljsbuild, that is.

This is a real issue for me as well, because it stops me from using core.async on AWS Lambda.

Show
Joel Wilsson added a comment - Stuart, could you please try the four lines from option 2 in my previous comment as a preamble and let me know if that solves the problem for you? Ie. put them in a file preamble.js on your project's classpath (I use resources/) and add :preamble "preamble.js" to your :compiler settings. Assuming that you are using lein-cljsbuild, that is. This is a real issue for me as well, because it stops me from using core.async on AWS Lambda.
Hide
David Nolen added a comment -

Sorry Stuart I'm not saying it's not real a issue but rather that I'm trying to determine the scope and precise nature of the issue. I noticed that the problem does not manifest when I use :optimizations :none. Are you and Joel using :simple, :advanced? Thanks.

Show
David Nolen added a comment - Sorry Stuart I'm not saying it's not real a issue but rather that I'm trying to determine the scope and precise nature of the issue. I noticed that the problem does not manifest when I use :optimizations :none. Are you and Joel using :simple, :advanced? Thanks.
Hide
Joel Wilsson added a comment -

I use :simple to get a single JavaScript file, which I can then use with AWS Lambda with this in main.js:

var r = require("./clojurescript-output.js");
exports.handler = r.hello_lambda.core.handler;

where clojurescript-output.js is generated from a file like

(ns hello_lambda.core)
(enable-console-print!)
(defn ^:export handler [event context]
  (println "Hello AWS Lambda")
  (.done context))

:advanced doesn't work for me: it just creates a JavaScript file with a single (function () {...})(), which makes it useless for creating a module (the symbols are not exported properly), but that's another issue.

Show
Joel Wilsson added a comment - I use :simple to get a single JavaScript file, which I can then use with AWS Lambda with this in main.js:
var r = require("./clojurescript-output.js");
exports.handler = r.hello_lambda.core.handler;
where clojurescript-output.js is generated from a file like
(ns hello_lambda.core)
(enable-console-print!)
(defn ^:export handler [event context]
  (println "Hello AWS Lambda")
  (.done context))
:advanced doesn't work for me: it just creates a JavaScript file with a single (function () {...})(), which makes it useless for creating a module (the symbols are not exported properly), but that's another issue.
Hide
David Nolen added a comment -

this was in fact an upstream ClojureScript issue, fixed as of this commit https://github.com/clojure/clojurescript/commit/e8ae2c8a9c3f2429555e136cc17c1669be5e993a

Show
David Nolen added a comment - this was in fact an upstream ClojureScript issue, fixed as of this commit https://github.com/clojure/clojurescript/commit/e8ae2c8a9c3f2429555e136cc17c1669be5e993a
Hide
Dusan Maliarik added a comment -

Not entirely fixed. If you use cljs.core.async/pub in the namespace,

eg. http://stackoverflow.com/questions/24698536/how-to-memoize-a-function-that-uses-core-async-and-non-blocking-channel-read/38069830#38069830
(then using it in some namespace)

(ns foo
  (:require ....))

(def foo
  (memoize-async
    (fn [...] ...)))

then goog.global.setTimeout is called before the cljs.nodejscli is evaled. Therefore causing

/home/skrat/Workspace/xxx/yyy.drie/bin/convert:37684
    goog.global.setTimeout(a, 0);
                ^

TypeError: goog.global.setTimeout is not a function
    at Function.setImmediate_ (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:37684:17)
    at Object.goog.async.nextTick (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:37630:270)
    at Object.cljs.core.async.impl.dispatch.queue_dispatcher (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:37718:21)
    at Object.cljs.core.async.impl.dispatch.run (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:37722:40)
    at Function.cljs.core.async.pub.cljs$core$IFn$_invoke$arity$3 (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:40775:33)
    at Function.cljs.core.async.pub.cljs$core$IFn$_invoke$arity$2 (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:40703:30)
    at Object.yyy.drie.io.async.memoize_async (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:42231:89)
    at Object.<anonymous> (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:59163:41)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)

I tried to require cljs.nodejscli in the ns that contains my main fn, but it results in

SEVERE: cljs.nodejscli:1: ERROR - namespace "cljs.nodejscli" cannot be provided twice
goog.provide('cljs.nodejscli');
^

Jul 12, 2016 2:34:34 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 1 error(s), 0 warning(s)
ERROR: JSC_DUPLICATE_NAMESPACE_ERROR. namespace "cljs.nodejscli" cannot be provided twice at cljs.nodejscli line 1 : 0
Show
Dusan Maliarik added a comment - Not entirely fixed. If you use cljs.core.async/pub in the namespace, eg. http://stackoverflow.com/questions/24698536/how-to-memoize-a-function-that-uses-core-async-and-non-blocking-channel-read/38069830#38069830 (then using it in some namespace)
(ns foo
  (:require ....))

(def foo
  (memoize-async
    (fn [...] ...)))
then goog.global.setTimeout is called before the cljs.nodejscli is evaled. Therefore causing
/home/skrat/Workspace/xxx/yyy.drie/bin/convert:37684
    goog.global.setTimeout(a, 0);
                ^

TypeError: goog.global.setTimeout is not a function
    at Function.setImmediate_ (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:37684:17)
    at Object.goog.async.nextTick (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:37630:270)
    at Object.cljs.core.async.impl.dispatch.queue_dispatcher (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:37718:21)
    at Object.cljs.core.async.impl.dispatch.run (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:37722:40)
    at Function.cljs.core.async.pub.cljs$core$IFn$_invoke$arity$3 (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:40775:33)
    at Function.cljs.core.async.pub.cljs$core$IFn$_invoke$arity$2 (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:40703:30)
    at Object.yyy.drie.io.async.memoize_async (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:42231:89)
    at Object.<anonymous> (/home/skrat/Workspace/xxx/yyy.drie/bin/convert:59163:41)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
I tried to require cljs.nodejscli in the ns that contains my main fn, but it results in
SEVERE: cljs.nodejscli:1: ERROR - namespace "cljs.nodejscli" cannot be provided twice
goog.provide('cljs.nodejscli');
^

Jul 12, 2016 2:34:34 PM com.google.javascript.jscomp.LoggerErrorManager printSummary
WARNING: 1 error(s), 0 warning(s)
ERROR: JSC_DUPLICATE_NAMESPACE_ERROR. namespace "cljs.nodejscli" cannot be provided twice at cljs.nodejscli line 1 : 0
Hide
Dusan Maliarik added a comment -

I believe the solution is to include cljs.nodejscli as soon as possible, instead of appending it at the end. This also means it has to be split into two pieces, one dealing with goog.global (included asap), and another one dealing with main-cli-fn (included at the end as is the case now).

Show
Dusan Maliarik added a comment - I believe the solution is to include cljs.nodejscli as soon as possible, instead of appending it at the end. This also means it has to be split into two pieces, one dealing with goog.global (included asap), and another one dealing with main-cli-fn (included at the end as is the case now).
Hide
Nick Alexander added a comment -

I am seeing this, with `optimizations :advanced` and `:simple`, with the following project https://github.com/ncalexan/datomish-user-agent-service/commit/af99ad3a16eece881a48fedd0b0f62517bb5cb25. It's not going to be 100% easy to duplicate, since an underlying library isn't published on Maven Central yet, but I can reproduce this consistently.

Any help?

Show
Nick Alexander added a comment - I am seeing this, with `optimizations :advanced` and `:simple`, with the following project https://github.com/ncalexan/datomish-user-agent-service/commit/af99ad3a16eece881a48fedd0b0f62517bb5cb25. It's not going to be 100% easy to duplicate, since an underlying library isn't published on Maven Central yet, but I can reproduce this consistently. Any help?
Hide
Nick Alexander added a comment -

For the next poor sod who runs into this, it is triggered by having a `go` at top-level.

Hat tip to user "Roc King" in the post http://clojure.2344290.n4.nabble.com/Will-advanced-optimizations-be-officially-supported-on-nodejs-tp7762p7855.html, who writes [sic]:

After got more familiar with clojurescript, I realized that ASYNC-110 fully described(and gave several working solutions!)[3] this issue.
But if "goog.global" was used before "goog.global = global"(the solution used in clojurescript[4]), the same problem will occur.
This is the exactly situation I have encountered several days ago: I wrote '(go (<! (timeout 1212)) ...)' in top level.

This was not clear to me until I read Roc's note. After I remove my top-level `go`, everything is happy.

Show
Nick Alexander added a comment - For the next poor sod who runs into this, it is triggered by having a `go` at top-level. Hat tip to user "Roc King" in the post http://clojure.2344290.n4.nabble.com/Will-advanced-optimizations-be-officially-supported-on-nodejs-tp7762p7855.html, who writes [sic]: After got more familiar with clojurescript, I realized that ASYNC-110 fully described(and gave several working solutions!)[3] this issue. But if "goog.global" was used before "goog.global = global"(the solution used in clojurescript[4]), the same problem will occur. This is the exactly situation I have encountered several days ago: I wrote '(go (<! (timeout 1212)) ...)' in top level. This was not clear to me until I read Roc's note. After I remove my top-level `go`, everything is happy.

People

  • Assignee:
    Unassigned
    Reporter:
    William
Vote (3)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved: