Module issues

[Kris Zyp]

A found a few issues with module loading. First, and most problematic, I
see modules being loaded multiple times:
http://gist.github.com/258778

Second, it seems that Node make some unnecessary accesses to the
values/objects in exports, and this can create infinite recursion that
can crash node:
http://gist.github.com/258792

Third, errors that are thrown from modules during loading do not
propagate across require calls (they just crash node):
http://gist.github.com/258779
Actually the require calls seem to fail to preserve normal call stack
semantics altogether, stack traces do not include information beyond the
require call at the top of the stack, the stack trace fails to provide
the rest of the stack that made the require call. I think the root cause
may be the promise wait function. The semantics of a promise wait
function (as used by require) are that it must throw an error when an
occurs in the promise. It doesn't seem that the promise wait function is
properly throwing the error.

On a related note, it would be nice if the promises in Node followed the
CommonJS proposal for promises:
http://gist.github.com/258809
Working with CommonJS promises and Node promises at the same time is a
bit of a headache, and this should be a pretty trivial addition.

Anyway, this is what I found trying to run Pintura on Node, and they are
showstoppers at the moment, but Node certainly looks promising, great
work on it.
Thanks,
Kris

[Ryan Dahl]

On Thu, Dec 17, 2009 at 4:28 PM, Kris Zyp <kri...@gmail.com> wrote:
> A found a few issues with module loading. First, and most problematic, I
> see modules being loaded multiple times:
> http://gist.github.com/258778
>
> Second, it seems that Node make some unnecessary accesses to the
> values/objects in exports, and this can create infinite recursion that
> can crash node:
> http://gist.github.com/258792
>
> Third, errors that are thrown from modules during loading do not
> propagate across require calls (they just crash node):
> http://gist.github.com/258779
> Actually the require calls seem to fail to preserve normal call stack
> semantics altogether, stack traces do not include information beyond the
> require call at the top of the stack, the stack trace fails to provide
> the rest of the stack that made the require call. I think the root cause
> may be the promise wait function. The semantics of a promise wait
> function (as used by require) are that it must throw an error when an
> occurs in the promise. It doesn't seem that the promise wait function is
> properly throwing the error.

Thanks for the bug reports. I'll try to fix them in a few days when I
have some time.

As for the last one, try throwing an instance of Error. For some
reason stack traces are not captured for arbitrary objects.

> On a related note, it would be nice if the promises in Node followed the
> CommonJS proposal for promises:
> http://gist.github.com/258809
> Working with CommonJS promises and Node promises at the same time is a
> bit of a headache, and this should be a pretty trivial addition.

The proposal is too simplistic and I don't like the method names. I
prefer Dojo's API with the ability to chain arguments and other
niceties. Node will be moving to that in the near future.

[Kris Zyp]

On Dec 17, 2:09 pm, Ryan Dahl <coldredle...@gmail.com> wrote:

> On Thu, Dec 17, 2009 at 4:28 PM, Kris Zyp <kris...@gmail.com> wrote:
> > A found a few issues with module loading. First, and most problematic, I
> > see modules being loaded multiple times:
> >http://gist.github.com/258778
>
> > Second, it seems that Node make some unnecessary accesses to the
> > values/objects in exports, and this can create infinite recursion that
> > can crash node:
> >http://gist.github.com/258792
>
> > Third, errors that are thrown from modules during loading do not
> > propagate across require calls (they just crash node):
> >http://gist.github.com/258779
> > Actually the require calls seem to fail to preserve normal call stack
> > semantics altogether, stack traces do not include information beyond the
> > require call at the top of the stack, the stack trace fails to provide
> > the rest of the stack that made the require call. I think the root cause
> > may be the promise wait function. The semantics of a promise wait
> > function (as used by require) are that it must throw an error when an
> > occurs in the promise. It doesn't seem that the promise wait function is
> > properly throwing the error.
>
> Thanks for the bug reports. I'll try to fix them in a few days when I
> have some time.
>
> As for the last one, try throwing an instance of Error. For some
> reason stack traces are not captured for arbitrary objects.

I don't understand, isn't new Error("some error") an instance of
(instanceof) Error?

>
> > On a related note, it would be nice if the promises in Node followed the
> > CommonJS proposal for promises:
> >http://gist.github.com/258809
> > Working with CommonJS promises and Node promises at the same time is a
> > bit of a headache, and this should be a pretty trivial addition.
>
> The proposal is too simplistic and I don't like the method names. I
> prefer Dojo's API with the ability to chain arguments and other
> niceties. Node will be moving to that in the near future.

Silently breaking compatibility with CommonJS due to personal naming
preference rather than open discussion is disappointing. I'd be happy
to discuss this on the commonjs list. And FWIW, Dojo 2.0 will probably
move to CommonJS's promise API, as we (I'm a Dojo committer) recognize
the shortcomings of their current API.

Anyway, I appreciate the help.
Thanks,
Kris

[Tim Caswell]

FYI, there has been much open discussion.

http://groups.google.com/group/nodejs/browse_frm/thread/4a1ce27a1f588a7f

> --
>
> You received this message because you are subscribed to the Google Groups "nodejs" group.
> To post to this group, send email to nod...@googlegroups.com.
> To unsubscribe from this group, send email to nodejs+un...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/nodejs?hl=en.
>
>

[Kris Zyp]

On Dec 17, 2:38 pm, Tim Caswell <t...@creationix.com> wrote:
> FYI, there has been much open discussion.
>
> http://groups.google.com/group/nodejs/browse_frm/thread/4a1ce27a1f588a7f

By open I mean in the context of where the proposal is made. Any
chance it could move to commonjs? Monitoring every single CommonJS
project's (there are numerous ones) mailing lists to carry out a
discussions on standards doesn't seem very reasonable.
Kris

[Felix Geisendörfer]

Hey Kris,

I'm the one who started to push for porting Dojo Deferred to node
since I think it is actually quite an awesome API. However, I am not
married to it. If you could summarize the problems you discovered with
it, that would be greatly appreciated.

I have almost completed porting Deferreds as a replacement for the
current Promise API of node here:

http://github.com/felixge/node/tree/deferred

You can skip most of the discussion Tim linked to. The main thing we
agreed on, is that we do not include Dojos automatic try..catch
behavior for callbacks. It will lead to code that is hard to debug and
silently fails. Instead we let any exception that is being thrown
bubble up and crash node, but handle return values that are
"instanceof Error" just the same as Dojo.

I did actually look at CommonJS promises and they seem flawed to me.
The API that I can see ( http://wiki.commonjs.org/wiki/Promises )
seems very limited. I also strongly agree with the idea of a progress
callback. I do understand that its useful, but Ryan pointed out one
thing in IRC to me that I became kind of obsessed with:

-> Promises are the non-blocking counterpart to functions

Ideally, functions are atomic operations, they do not report progress.
If you need progress callbacks, pass in a callback for that to the
function that gives the promise. I do like that chaining offered by
Deferreds, but to me that is not in violation with the concept of a
non-blocking function. To me that is merely a convenient way to
compose new functions from a series of blocking/non-blocking
functions.

Anyway, we might want to start a new thread for this. If you'd rather
have the discussion over at CommonJS, I can start a thread there. Just
let me know what I've missed so far, so I can explain my point of view
in respect to that.

Thanks,
-- Felix Geisendörfer aka the_undefined

[Ryan Dahl]

On Thu, Dec 17, 2009 at 10:21 PM, Kris Zyp <kri...@gmail.com> wrote:
>> > Third, errors that are thrown from modules during loading do not
>> > propagate across require calls (they just crash node):
>> >http://gist.github.com/258779
>> > Actually the require calls seem to fail to preserve normal call stack
>> > semantics altogether, stack traces do not include information beyond the
>> > require call at the top of the stack, the stack trace fails to provide
>> > the rest of the stack that made the require call. I think the root cause
>> > may be the promise wait function. The semantics of a promise wait
>> > function (as used by require) are that it must throw an error when an
>> > occurs in the promise. It doesn't seem that the promise wait function is
>> > properly throwing the error.
>>
>> Thanks for the bug reports. I'll try to fix them in a few days when I
>> have some time.
>>
>> As for the last one, try throwing an instance of Error. For some
>> reason stack traces are not captured for arbitrary objects.
>
> I don't understand, isn't new Error("some error") an instance of
> (instanceof) Error?

Oops, sorry - I didn't catch that in your gist. You're right - bug.

[Isaac Z. Schlueter]

The multiple-loading module is (I believe) related to the way that the
module IDs are constructed with relative paths containing "..". I'd
be curious if you could reproduce that with the code here:
http://github.com/isaacs/node/tree/dotdotmodulepath

The unnecessary access looks like it's happening in a debug()
statement in node.js. I'm playing around now in my debug-revamp
branch with a way to have the same level of debugging without
introducing this weirdness in cases where the debug flag isn't turned
on.

I don't have a fix for the promise Error stack thing, but I'm assuming
Ryan will have an easier time with that when he gets to it.

Anyone think we should start using github's issues for more of this
stuff, or stick with the "strictly list" style of bug management? The
nice thing about using a mailing list and IRC as the bug tracker is
that unimportant bugs never waste anyone's time (no oil for the wheels
that don't squeak.) Otoh, it can be nice to have a list of known
things for people to pick from if they feel like helping out.

As for the naming around modules and promises, I think that's a one
for #commonjs to fight over. As long as the API has a lot of support
behind it, and there's no harm to nodejs to follow the crowd, I
generally think we should just implement the spec. That seems to be
the way that nodejs has gone in the past (moving from import() to
require(), for instance). Currently, it seems like the promise
proposal is still a bit in flux, so imo, maybe it's better to just
leave things as they are. (I could be wrong on that, are other folks
implementing it according to the proposal? Does it have critical mass
yet?)

--i

On Dec 17, 3:27 pm, Ryan Dahl <coldredle...@gmail.com> wrote:

[ry]

On Dec 17, 4:28 pm, Kris Zyp <kris...@gmail.com> wrote:
> Third, errors that are thrown from modules during loading do not
> propagate across require calls (they just crash node):http://gist.github.com/258779
> Actually the require calls seem to fail to preserve normal call stack
> semantics altogether, stack traces do not include information beyond the
> require call at the top of the stack, the stack trace fails to provide
> the rest of the stack that made the require call. I think the root cause
> may be the promise wait function. The semantics of a promise wait
> function (as used by require) are that it must throw an error when an
> occurs in the promise. It doesn't seem that the promise wait function is
> properly throwing the error.

Fixed in bfa3613.

[Kris Zyp]

Now it seems that Node doesn't print out anything (no stack trace at
all), when an uncaught exception is thrown. If I just run a script
like:
throw new Error("anything")
Node just immediately returns, no error messages or anything.
Also, I can't seem to load modules with "../module" at all now:
http://gist.github.com/261533
And it also seems that I can't add relative paths to require.paths
anymore.
Kris

[Daniel N]

I'm unable to load relative paths that go up a directory and back down from the script.

eg:

examples

  - simple.js

lib

  - foo.js

# simple.js

var sys = require('sys')

try{

  require('./../lib/foo')

} catch(e) {

  sys.puts(e.stack)

}

sys.puts("Yay It loaded")

///////////////

Gives me:

example$ node simple.js

Error: Cannot find module './../lib/foo'

    at node.js:813:31

    at findModulePath (node.js:744:5)

    at node.js:766:7

    at node.js:775:7

    at [object Object].<anonymous> (node.js:712:32)

    at [object Object].emitError (node.js:266:13)

    at node.js:505:27

    at [object Object].wait (node.js:324:11)

    at require (node.js:895:32)

This issue seems to have been introduced in

c2454a0d067d1587f7fcc529543e521cc73620f0

I'd really love to get a fix in for this but I'm afraid my fu isn't yet strong enough to even get the debugger to print the debug statements :'(

Cheers

Daniel

[Micheil Smith]

Hi Daniel,

I'm pretty sure Isaac's patch fixes this issue. Please try with his patches: http://github.com/isaacs/node/tree/dotdotmodulepath

- Micheil.