Missing assert option

[dev-null-undefined]

If I am not mistaking the NGX_ABORT value is also valid for the rc here:

ngx_wasm_module/src/wasm/ngx_wasm_ops.c

Line 563 in 883b627

ngx_wa_assert(rc == NGX_OK /* step completed */

Since it can be returned from ngx_proxy_wasm_resume on wasmtime trap?
And after returning from this function you are also taking that value into count. So I think you forgot it here.

[thibaultcha]

It does not seem true to me. On instance trap ngx_proxy_wasm_resume will return NGX_ERROR or NGX_HTTP_INTERNAL_SERVER_ERROR as per action2rc.

[dev-null-undefined]

I am bit confused then by this if and others that are used whenever handling rc from ngx_wavm_instance_call_funcref

ngx_wasm_module/src/http/proxy_wasm/ngx_http_proxy_wasm.c

Line 43 in 883b627

if (rc == NGX_ERROR || rc == NGX_ABORT) {

from there and many other places it seems to me like the value NGX_ABORT (-6) is allowed return value that is counted upon.

On that note from the assert (

ngx_wasm_module/src/wasm/ngx_wasm_ops.c

Line 568 in 883b627

|| rc == NGX_AGAIN /* step yielded */

)

I can see NGX_AGAIN being mentined as a way to yield a step?
But I can not find it anywhere else in the code.

[thibaultcha]

A simple way to test return status codes is to add an assertion and run the whole test suite. When a test fails because of the assertion, you then know that you can isolate that specific test to further debug the case.

[thibaultcha]

The first link in your comment (ngx_http_proxy_wasm.c) does need to cover NGX_ABORT because it calls the low-level ngx_wavm_instance_call_funcref which directly interacts with the Wasm VM. The second link is indeed for yielding which is communicated via a subsystem-agnostic state machine (env->state) see ngx_wasm_op_call_handler.