the Function() method stub returns invalid value

[drsm]

while the Function() constructor is a stub, it should throw InternalError, i think.
and there is problem around it and maybe in other places aswell

>> new Function
undefined
    at native (native)
    at main (native)

>> throw new Error('test')
Error: test
    at main (native)

>> new Function
Error: test
    at native (native)
    at main (native)

does this patch resolve the problem or just hide it?

# HG changeset patch
# User Artem S. Povalyukhin <artem.povaluhin@gmail.com>
# Date 1532731490 -10800
#      Sat Jul 28 01:44:50 2018 +0300
# Node ID a8811f27d1d0c976734ab20743d9121ed10be3dc
# Parent  058162fce59aa3136de927db6246a6a4d5b7e025
Fixed return value of Function() method stub.

diff -r 058162fce59a -r a8811f27d1d0 njs/njs_function.c
--- a/njs/njs_function.c        Fri Jul 27 17:01:52 2018 +0300
+++ b/njs/njs_function.c        Sat Jul 28 01:44:50 2018 +0300
@@ -457,6 +457,8 @@
 njs_function_constructor(njs_vm_t *vm, njs_value_t *args, nxt_uint_t nargs,
     njs_index_t unused)
 {
+    njs_internal_error(vm, "Not implemented");
+
     return NXT_ERROR;
 }

[xeioex]

Yes, the patch resolves the issue for Function().

The general problem is that return value of native function like njs_function_constructor() (NXT_ERROR, NXT_OK) is treated separately from the VM return value (vm->retval). njs shell uses njs_vm_value_dump() to dump vm->retval. njs_vm_init() sets vm->retval to undefined. So if a piece of code returns NXT_ERROR, but does not set vm->retval

undefined
    ...
    at main (native)

will be printed

[drsm]

Hi @xeioex!
but why it throws the previously thrown exception, or use an argument as an exception...

I'm mostly bothering about this:

>> try { var a = Array(1111111111); } catch(e) { console.log(typeof e, e); }
'number' 1111111111
undefined

[xeioex]

Array constructor is expected to overwrite vm->retval with an exception value if an exception happens. Currently it does not do it (this is the main culprit). vm->retval is used everywhere inside the VM as an intermediate value. So, most of the time it stores the value of the previous instruction.

./build/njs -d can show you the instructions

>> try { var a = Array(1111111111); } catch(e) { console.log(typeof e, e); }
00000 TRY START         7FCD54033AC0 +144
00032 FUNCTION FRAME    0011 1
# https://github.com/nginx/njs/blob/master/njs/njs_vm.c#L1923
00064 MOVE              0002 7FCD54033AD0  <= moving 1111111111 constant into Array() function frame and into vm->retval 
00096 FUNCTION CALL     7FCD54033AA0 <= invoking Array()
00120 TRY END           +184
00144 CATCH             7FCD54033AB0 +32
00176 METHOD FRAME      7FCD54000C40 7FCD54033AE0 2
00216 TYPEOF            0002 7FCD54033AB0
00248 MOVE              0012 7FCD54033AB0
00280 FUNCTION CALL     7FCD54033AF0
00304 STOP

number 1111111111
undefined

1. When FUNCTION CALL instruction fails with NXT_ERROR backtrace is set (https://github.com/nginx/njs/blob/master/njs/njs_vm.c#L212)

2. call stack is unwound until catch block is found or the top frame is reached (https://github.com/nginx/njs/blob/master/njs/njs_vm.c#L214)

3. if catch block is found, backtrace is reset (https://github.com/nginx/njs/blob/master/njs/njs_vm.c#L222)

4. catch block header expects to see the exception value in vm->reval
   (https://github.com/nginx/njs/blob/master/njs/njs_vm.c#L2790)

In the end the e argument of the catch block contains vm->retval value (which is the value of the last successful instruction (MOVE), which is 1111111111 number value)

[drsm]

Thank you for an explanation, it's completely clear now!