From 825cfd67266444a20b6ff08902cdf546723ab3c8 Mon Sep 17 00:00:00 2001 From: Crypt Keeper <64215+codefromthecrypt@users.noreply.github.com> Date: Sun, 4 Jun 2023 19:38:05 +0800 Subject: [PATCH] Backfills tests for guest panics (#73) This backfills tests for guests that panic. Sadly, we don't control the ModuleConfig. Integrators who want to see the data written by TinyGo during a panic need to capture stdout on their own. This is already the case in dapr, which sends stdout to the logger under debug. Signed-off-by: Adrian Cole --- handler/middleware_test.go | 101 +++++++++++++++--- internal/test/testdata.go | 13 ++- .../error/panic_on_handle_request.wasm | Bin 0 -> 315 bytes .../error/panic_on_handle_request.wat | 31 ++++++ .../error/panic_on_handle_response.wasm | Bin 0 -> 319 bytes .../error/panic_on_handle_response.wat | 32 ++++++ .../test/testdata/error/panic_on_start.wasm | Bin 0 -> 333 bytes .../test/testdata/error/panic_on_start.wat | 35 ++++++ .../error/set_request_header_after_next.wasm | Bin 0 -> 401 bytes .../set_request_header_after_next.wat | 4 +- .../set_request_header_after_next.wasm | Bin 368 -> 0 bytes 11 files changed, 199 insertions(+), 17 deletions(-) create mode 100644 internal/test/testdata/error/panic_on_handle_request.wasm create mode 100644 internal/test/testdata/error/panic_on_handle_request.wat create mode 100644 internal/test/testdata/error/panic_on_handle_response.wasm create mode 100644 internal/test/testdata/error/panic_on_handle_response.wat create mode 100644 internal/test/testdata/error/panic_on_start.wasm create mode 100644 internal/test/testdata/error/panic_on_start.wat create mode 100644 internal/test/testdata/error/set_request_header_after_next.wasm rename internal/test/testdata/{invalid => error}/set_request_header_after_next.wat (83%) delete mode 100644 internal/test/testdata/invalid/set_request_header_after_next.wasm diff --git a/handler/middleware_test.go b/handler/middleware_test.go index daf8d67..213f90d 100644 --- a/handler/middleware_test.go +++ b/handler/middleware_test.go @@ -4,7 +4,6 @@ import ( "context" _ "embed" "reflect" - "strings" "testing" "github.com/http-wasm/http-wasm-host-go/api/handler" @@ -13,16 +12,89 @@ import ( var testCtx = context.Background() -func TestMiddlewareAfterNextErrors(t *testing.T) { +func TestNewMiddleware(t *testing.T) { tests := []struct { name string guest []byte expectedError string }{ { - name: "set_header_value request", - guest: test.BinInvalidSetRequestHeaderAfterNext, - expectedError: "can't set request header after next handler", + name: "ok", + guest: test.BinE2EProtocolVersion, + }, + { + name: "panic on _start", + guest: test.BinErrorPanicOnStart, + expectedError: `wasm: error instantiating guest: module[1] function[_start] failed: wasm error: unreachable +wasm stack trace: + panic_on_start.main()`, + }, + } + + for _, tt := range tests { + tc := tt + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + mw, err := NewMiddleware(testCtx, tc.guest, handler.UnimplementedHost{}) + requireEqualError(t, err, tc.expectedError) + if mw != nil { + mw.Close(testCtx) + } + }) + } +} + +func TestMiddlewareHandleRequest_Error(t *testing.T) { + tests := []struct { + name string + guest []byte + expectedError string + }{ + { + name: "panic", + guest: test.BinErrorPanicOnHandleRequest, + expectedError: `wasm error: unreachable +wasm stack trace: + panic_on_handle_request.handle_request() i64`, + }, + } + + for _, tt := range tests { + tc := tt + t.Run(tc.name, func(t *testing.T) { + mw, err := NewMiddleware(testCtx, tc.guest, handler.UnimplementedHost{}) + if err != nil { + t.Fatal(err) + } + defer mw.Close(testCtx) + + _, _, err = mw.HandleRequest(testCtx) + requireEqualError(t, err, tc.expectedError) + }) + } +} + +func TestMiddlewareHandleResponse_Error(t *testing.T) { + tests := []struct { + name string + guest []byte + expectedError string + }{ + { + name: "panic", + guest: test.BinErrorPanicOnHandleResponse, + expectedError: `wasm error: unreachable +wasm stack trace: + panic_on_handle_response.handle_response(i32,i32)`, + }, + { + name: "set_header_value request", + guest: test.BinErrorSetRequestHeaderAfterNext, + expectedError: `can't set request header after next handler (recovered by wazero) +wasm stack trace: + http_handler.set_header_value(i32,i32,i32,i32,i32) + set_request_header_after_next.handle_response(i32,i32)`, }, } @@ -41,18 +113,11 @@ func TestMiddlewareAfterNextErrors(t *testing.T) { // We do expect an error on the response path err = mw.HandleResponse(ctx, 0, nil) - requireErrorPrefix(t, err, tc.expectedError) + requireEqualError(t, err, tc.expectedError) }) } } -func requireErrorPrefix(t *testing.T, err error, want string) { - t.Helper() - if have := err.Error(); !strings.HasPrefix(have, want) { - t.Errorf("unexpected error message prefix, want: %s, have: %s", want, have) - } -} - func TestMiddlewareResponseUsesRequestModule(t *testing.T) { mw, err := NewMiddleware(testCtx, test.BinE2EHandleResponse, handler.UnimplementedHost{}) if err != nil { @@ -132,3 +197,13 @@ func requireHandleRequest(t *testing.T, mw Middleware, ctxNext handler.CtxNext, t.Error("expected handler to not return guest to the pool") } } + +func requireEqualError(t *testing.T, err error, expectedError string) { + if err != nil { + if want, have := expectedError, err.Error(); want != have { + t.Fatalf("unexpected error: want %v, have %v", want, have) + } + } else if want := expectedError; want != "" { + t.Fatalf("expected error %v", want) + } +} diff --git a/internal/test/testdata.go b/internal/test/testdata.go index c6e88c1..987be23 100644 --- a/internal/test/testdata.go +++ b/internal/test/testdata.go @@ -86,8 +86,17 @@ var BinE2EHandleResponse []byte //go:embed testdata/e2e/header_names.wasm var BinE2EHeaderNames []byte -//go:embed testdata/invalid/set_request_header_after_next.wasm -var BinInvalidSetRequestHeaderAfterNext []byte +//go:embed testdata/error/panic_on_handle_request.wasm +var BinErrorPanicOnHandleRequest []byte + +//go:embed testdata/error/panic_on_handle_response.wasm +var BinErrorPanicOnHandleResponse []byte + +//go:embed testdata/error/panic_on_start.wasm +var BinErrorPanicOnStart []byte + +//go:embed testdata/error/set_request_header_after_next.wasm +var BinErrorSetRequestHeaderAfterNext []byte // binExample instead of go:embed as files aren't relative to this directory. func binExample(name string) []byte { diff --git a/internal/test/testdata/error/panic_on_handle_request.wasm b/internal/test/testdata/error/panic_on_handle_request.wasm new file mode 100644 index 0000000000000000000000000000000000000000..14efddc9f72368b3b3fe9c75fecc79a3102d673c GIT binary patch literal 315 zcmZ9H!A`?45JYF!CPY{i1QJx-5N9g&$}QqU9IM%?mJ%n}X#*U{AMnkjk?5f#%^v13 z`vxvG1c2^TsmDn)mY|=K#}WKO4}(kIrr^4?YkS*8Id~b?`L?n{^t}K|G4X|_NtdrG zYoux87h|z=p{m73`PfV9K{ux$b!|ujzPjfH3Myvb;t7Uf%u=C%p&W%al%dE?*16y} z&-f)BTq9UM&Vg-%J$24cXsvL41YZ5)(HRDxQ}COD25&3UzC9$)kJr``l!=n|b-zmf TTL{d^rYZ0JC*?j_iLs4;1FTh7 literal 0 HcmV?d00001 diff --git a/internal/test/testdata/error/panic_on_handle_request.wat b/internal/test/testdata/error/panic_on_handle_request.wat new file mode 100644 index 0000000..3305bdd --- /dev/null +++ b/internal/test/testdata/error/panic_on_handle_request.wat @@ -0,0 +1,31 @@ +;; panic_on_handle_request issues an unreachable instruction after writing +;; an error to stdout. This simulates a panic in TinyGo. +(module $panic_on_handle_request + ;; Import the fd_write function from wasi, used in TinyGo for println. + (import "wasi_snapshot_preview1" "fd_write" + (func $wasi.fd_write (param $fd i32) (param $iovs i32) (param $iovs_len i32) (param $result.size i32) (result (;errno;) i32))) + + ;; Allocate the minimum amount of memory, 1 page (64KB). + (memory (export "memory") 1 1) + + ;; Pre-populate memory with the panic message, in iovec format + (data (i32.const 0) "\08") ;; iovs[0].offset + (data (i32.const 4) "\06") ;; iovs[0].length + (data (i32.const 8) "panic!") ;; iovs[0] + + ;; On handle_request, write "panic!" to stdout and crash. + (func $handle_request (export "handle_request") (result (; ctx_next ;) i64) + ;; Write the panic to stdout via its iovec [offset, len]. + (call $wasi.fd_write + (i32.const 1) ;; stdout + (i32.const 0) ;; where's the iovec + (i32.const 1) ;; only one iovec + (i32.const 0) ;; overwrite the iovec with the ignored result. + ) + drop ;; ignore the errno returned + + ;; Issue the unreachable instruction instead of returning ctx_next + (unreachable)) + + (func $handle_response (export "handle_response") (param $reqCtx i32) (param $is_error i32)) +) diff --git a/internal/test/testdata/error/panic_on_handle_response.wasm b/internal/test/testdata/error/panic_on_handle_response.wasm new file mode 100644 index 0000000000000000000000000000000000000000..6e576105585682b99868a9c8a98dd49a84dfb040 GIT binary patch literal 319 zcmZ9HF;BxV6ol_RyCK4&MGRG8L(Ej_$`<$`j@7)XmJ%n}X#))8C-9d^E5Xo{?uHvq z-@&DZ0MMN(^*D*f67*g2ID(&OIk@C)3a(50wzplBqnBZm?HB?-SMXz;Eg?b~DG{FJtqpiGo>sQY#D UUqWC;Hr;vaKPdOfN{ns%1qz~87ytkO literal 0 HcmV?d00001 diff --git a/internal/test/testdata/error/panic_on_handle_response.wat b/internal/test/testdata/error/panic_on_handle_response.wat new file mode 100644 index 0000000..17615a2 --- /dev/null +++ b/internal/test/testdata/error/panic_on_handle_response.wat @@ -0,0 +1,32 @@ +;; panic_on_handle_response issues an unreachable instruction after writing +;; an error to stdout. This simulates a panic in TinyGo. +(module $panic_on_handle_response + ;; Import the fd_write function from wasi, used in TinyGo for println. + (import "wasi_snapshot_preview1" "fd_write" + (func $wasi.fd_write (param $fd i32) (param $iovs i32) (param $iovs_len i32) (param $result.size i32) (result (;errno;) i32))) + + ;; Allocate the minimum amount of memory, 1 page (64KB). + (memory (export "memory") 1 1) + + ;; Pre-populate memory with the panic message, in iovec format + (data (i32.const 0) "\08") ;; iovs[0].offset + (data (i32.const 4) "\06") ;; iovs[0].length + (data (i32.const 8) "panic!") ;; iovs[0] + + (func $handle_request (export "handle_request") (result (; ctx_next ;) i64) + (return (i64.const 1))) ;; call the next handler + + ;; On handle_response, write "panic!" to stdout and crash. + (func $handle_response (export "handle_response") (param $reqCtx i32) (param $is_error i32) + ;; Write the panic to stdout via its iovec [offset, len]. + (call $wasi.fd_write + (i32.const 1) ;; stdout + (i32.const 0) ;; where's the iovec + (i32.const 1) ;; only one iovec + (i32.const 0) ;; overwrite the iovec with the ignored result. + ) + drop ;; ignore the errno returned + + ;; Issue the unreachable instruction instead of returning. + (unreachable)) +) diff --git a/internal/test/testdata/error/panic_on_start.wasm b/internal/test/testdata/error/panic_on_start.wasm new file mode 100644 index 0000000000000000000000000000000000000000..049bde66ef274c55ad691504d60733029ed6ad6d GIT binary patch literal 333 zcmZ9Hu};G<7=-V)-4J1svVedMu~%Y&g(dJHjz#{emJ&PIZUYSD33zAHA{cto-Ehl& z2b+2VKo_b}(=3`ApaFd~VwxcC=yJ5lndI$|HX)lK^3L&iU#?p-Mwc0oN|KNZMYB#G zRMB%E;)g(CQnoRJs*Uw+$0qXImQ#jU{ugNoJ~8C!wX9H6eegMMpci%MV literal 0 HcmV?d00001 diff --git a/internal/test/testdata/error/panic_on_start.wat b/internal/test/testdata/error/panic_on_start.wat new file mode 100644 index 0000000..32e5b51 --- /dev/null +++ b/internal/test/testdata/error/panic_on_start.wat @@ -0,0 +1,35 @@ +;; panic_on_start is a WASI command which issues an unreachable instruction +;; after writing an error to stdout. This simulates a panic in TinyGo. +(module $panic_on_start + ;; Import the fd_write function from wasi, used in TinyGo for println. + (import "wasi_snapshot_preview1" "fd_write" + (func $wasi.fd_write (param $fd i32) (param $iovs i32) (param $iovs_len i32) (param $result.size i32) (result (;errno;) i32))) + + ;; Allocate the minimum amount of memory, 1 page (64KB). + (memory (export "memory") 1 1) + + ;; Pre-populate memory with the panic message, in iovec format + (data (i32.const 0) "\08") ;; iovs[0].offset + (data (i32.const 4) "\06") ;; iovs[0].length + (data (i32.const 8) "panic!") ;; iovs[0] + + ;; On start, write "panic!" to stdout and crash. + (func $main (export "_start") + ;; Write the panic to stdout via its iovec [offset, len]. + (call $wasi.fd_write + (i32.const 1) ;; stdout + (i32.const 0) ;; where's the iovec + (i32.const 1) ;; only one iovec + (i32.const 0) ;; overwrite the iovec with the ignored result. + ) + drop ;; ignore the errno returned + + ;; Issue the unreachable instruction instead of exiting normally + (unreachable)) + + ;; Export the required functions for the handler ABI + (func $handle_request (export "handle_request") (result (; ctx_next ;) i64) + (return (i64.const 0))) ;; don't call the next handler + + (func $handle_response (export "handle_response") (param $reqCtx i32) (param $is_error i32)) +) diff --git a/internal/test/testdata/error/set_request_header_after_next.wasm b/internal/test/testdata/error/set_request_header_after_next.wasm new file mode 100644 index 0000000000000000000000000000000000000000..e770c080ca4d686b9e476c7d3a0f30d7a8df2075 GIT binary patch literal 401 zcmZXQK~BRk5JhLkPFs?wjgVjmQk8YZh6|Klfqh~rgA$QA!FEx*p@-vQ*a@^!nZ@^L z*%dXbicRsU+C2jZK}eBAU$Z-T zf-NLo(jZxoIGJz6tmCdXM}p;KR2cr)vvq*0^S}CQv@%#N|xR$ha7XtMFvT- zreL1#dhJ-d&FgXCkP$mRxTm32jfQWOT6GMM4*~Qy}v^*r$mW3RxyjZ~vnv}s8)v|ZS{@T4RyYzKV!Y+3$( z49fNh0C+2^=o|_vz^{t@KpV^&=f<*8y6)KImR;Gfs@asEsyi@%5QG#-^cCOBGwdMs zkOjyCBh`KoPv)I$e3K0tzgs5ILmjhJtvu zAGBlbwy&p=gGb`{=$^+;wHm%rYSlA5J%*saCVzp|oAa@?zs5U+f3MI^Km_u=)%C1G r%b-