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 <[email protected]>
http-wasm · Jun 4, 2023 · 825cfd6 · 825cfd6
1 parent 0368c00
commit 825cfd6
Show file tree

Hide file tree

Showing 11 changed files with 199 additions and 17 deletions.
diff --git 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
@@ -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
diff --git a/internal/test/testdata/error/panic_on_handle_request.wat 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
diff --git a/internal/test/testdata/error/panic_on_handle_response.wat 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
diff --git a/internal/test/testdata/error/panic_on_start.wat 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
diff --git a/...invalid/set_request_header_after_next.wat → ...a/error/set_request_header_after_next.wat b/...invalid/set_request_header_after_next.wat → ...a/error/set_request_header_after_next.wat
@@ -15,12 +15,12 @@
   (global $value_len i32 (i32.const 10))
 
   ;; handle_request returns non-zero to proceed to the next handler.
-  (func (export "handle_request") (result (; ctx_next ;) i64)
+  (func $handle_request (export "handle_request") (result (; ctx_next ;) i64)
     ;; call the next handler
     (return (i64.const 1)))
 
   ;; handle_response tries to set a request header even though it is too late.
-  (func (export "handle_response") (param $reqCtx i32) (param $is_error i32)
+  (func $handle_response (export "handle_response") (param $reqCtx i32) (param $is_error i32)
     (call $set_header_value
       (i32.const 0) ;; header_kind_request
       (global.get $name) (global.get $name_len)

diff --git a/internal/test/testdata/invalid/set_request_header_after_next.wasm b/internal/test/testdata/invalid/set_request_header_after_next.wasm