Stack traces loaded through require() always show duk_bi_global.c as source file

[emil-e]

duk_bi_global_object_require uses the function duk_eval (duk_bi_global.c:1079) to compile the source returned by the modSearch function. Since duk_eval is defined as,

#define duk_eval(ctx)  \
    ((void) duk_push_string((ctx), __FILE__), \
    (void) duk_eval_raw((ctx), NULL, 0, DUK_COMPILE_EVAL))

the stack frames for functions loaded through require(...) will show the source file as duk_bi_global.c.

I came up with the following fix (replace duk_bi_global.c:1079) but I don't know if this is the "right" solution or not:

duk_dup(ctx, 3);
duk_eval_raw(ctx, NULL, 0, DUK_COMPILE_EVAL);

This uses the resolved module ID as the filename for duk_eval_raw instead.

[andoma]

This is definitely an improvement over the current solution which results in very confusing backtraces.

[svaarala]

Looks definitely like something to fix, thanks for reporting it.

[svaarala]

Technically it might be useful if the modSearch() function could provide the filename if the file actually gets loaded from a file system (not always the case but quite common).

Unfortunately this isn't easily fixable for 1.x because but we can look into changing the contract (@creationix had also some issues with the current one, specifically re: giving a fully resolved ID to modSearch) in 2.0.

[svaarala]

Ah, I see. The duk_bi_global_object_require() already does the following which spot fixes the fileName for the module function itself, but that of course doesn't get "inherited" into functions defined within the module:

        /* Finish the wrapped module source. */
        duk_push_string(ctx, "})");
        duk_concat(ctx, 3);
        duk_eval(ctx);

        /* Force 'fileName' property of the module function so that if the
         * module creates a logger, the logger name defaults to the module
         * name.
         */
        duk_dup(ctx, 3);
        duk_put_prop_stridx(ctx, -2, DUK_STRIDX_FILE_NAME);

The proposed change fixes the issue and also removes the unnecessary forcing of the fileName, which is nice. I'll merge a fix soon.

[svaarala]

The merge should fix the issue, and the following testcase was added to check for the preferred behavior at this point:

19491ef

[svaarala]

@emil-e It'd be nice to hear if the merge (in master) fixed this issue for you. Quite probably so because the fix is practically the one you suggested originally, but still :)

[emil-e]

I'll check it out :)

On Thursday, October 30, 2014, Sami Vaarala notifications@github.com
wrote:

@emil-e https://github.com/emil-e It'd be nice to hear if the merge (in
master) fixed this issue for you. Quite probably so because the fix is
practically the one you suggested originally, but still :)

—
Reply to this email directly or view it on GitHub
#58 (comment).

[svaarala]

This is now also fixed in 1.0.1 release.

[creationix]

Awesome thanks. I wanted this patch, but I'm trying to be good and stick to released versions.

[andoma]

FWIW, I use 1.0.1 in https://github.com/andoma/showtime/ and there it now prints backtraces as it should.

[svaarala]

@emil-e I'll close this now because it seems to be fixed - if not, please reopen.

[emil-e]

Sorry for the delay, I haven't had time to test it since I've been
traveling but I can't imagine this not working. Andoma tested it and it
worked for him so I'm perfectly fine with closing. Thanks for the great
response!

On Friday, November 7, 2014, Sami Vaarala notifications@github.com wrote:

Closed #58 #58.

—
Reply to this email directly or view it on GitHub
#58 (comment).