From c250e5c5475fa865ee4a6e8702beafc621ff51be Mon Sep 17 00:00:00 2001 From: Carlos Nihelton Date: Mon, 26 Aug 2024 16:42:10 -0300 Subject: [PATCH] Test the store wrapper error translation There is some non-negligible error translation in there mostly due the somewhat involving error handling imposed by syscall/dll_windows. This adds a modest set of test cases to cover that logic. For that to be approachable I extracted the error reporting out of the proc call method, this way we don't depend on calling a DLL function to test this part. Quite artificial setup, but I always enjoy separating "computing" from "doing". --- .../go-wrapper/microsoftstore/export_test.go | 3 ++ storeapi/go-wrapper/microsoftstore/store.go | 28 ++++++++++++++++ .../go-wrapper/microsoftstore/store_test.go | 32 +++++++++++++++++++ .../microsoftstore/store_windows.go | 25 +-------------- 4 files changed, 64 insertions(+), 24 deletions(-) diff --git a/storeapi/go-wrapper/microsoftstore/export_test.go b/storeapi/go-wrapper/microsoftstore/export_test.go index d893d5df4..ee7467f26 100644 --- a/storeapi/go-wrapper/microsoftstore/export_test.go +++ b/storeapi/go-wrapper/microsoftstore/export_test.go @@ -2,3 +2,6 @@ package microsoftstore // FindWorkspaceRoot climbs up the current working directory until the Go workspace root is found. var FindWorkspaceRoot = findWorkspaceRoot + +// CheckError inspects the values of hres and err to determine what kind of error we have, if any, according to the rules of syscall/dll_windows.go. +var CheckError = checkError diff --git a/storeapi/go-wrapper/microsoftstore/store.go b/storeapi/go-wrapper/microsoftstore/store.go index ef792e710..ef1698354 100644 --- a/storeapi/go-wrapper/microsoftstore/store.go +++ b/storeapi/go-wrapper/microsoftstore/store.go @@ -2,8 +2,10 @@ package microsoftstore import ( "errors" + "fmt" "os" "path/filepath" + "syscall" ) // findWorkspaceRoot climbs up the current working directory until the Go workspace root is found. @@ -26,3 +28,29 @@ func findWorkspaceRoot() (string, error) { } } } + +// checkError inspects the values of hres and err to determine what kind of error we have, if any, according to the rules of syscall/dll_windows.go. +func checkError(hres int64, err error) (int64, error) { + // From syscall/dll_windows.go (*Proc).Call doc: + // > Callers must inspect the primary return value to decide whether an + // error occurred [...] before consulting the error. + if e := NewStoreAPIError(hres); e != nil { + return hres, fmt.Errorf("storeApi returned error code %d: %w", hres, e) + } + + if err == nil { + return hres, nil + } + + var target syscall.Errno + if b := errors.As(err, &target); !b { + // Supposedly unrechable: proc.Call must always return a syscall.Errno + return hres, err + } + + if target != syscall.Errno(0) { + return hres, fmt.Errorf("failed syscall to storeApi: %v (syscall errno %d)", target, err) + } + + return hres, nil +} diff --git a/storeapi/go-wrapper/microsoftstore/store_test.go b/storeapi/go-wrapper/microsoftstore/store_test.go index b502a797b..ea7684603 100644 --- a/storeapi/go-wrapper/microsoftstore/store_test.go +++ b/storeapi/go-wrapper/microsoftstore/store_test.go @@ -2,12 +2,14 @@ package microsoftstore_test import ( "context" + "errors" "fmt" "log/slog" "os" "os/exec" "path/filepath" "runtime" + "syscall" "testing" "time" @@ -87,6 +89,36 @@ func TestGetSubscriptionExpirationDate(t *testing.T) { require.ErrorIs(t, gotErr, wantErr, "GetSubscriptionExpirationDate should have returned code %d", wantErr) } +func TestErrorVerification(t *testing.T) { + t.Parallel() + testcases := map[string]struct { + hresult int64 + err error + + wantError bool + }{ + "Success": {}, + // If HRESULT is not in the Store API error range and err is not a syscall.Errno then we don't have an error. + "With an unknown value (not an error)": {hresult: 1, wantError: false}, + + "Upper bound of the Store API enum range": {hresult: -1, wantError: true}, + "Lower bound of the Store API enum range": {hresult: int64(microsoftstore.ErrNotSubscribed), wantError: true}, + "With a system error (errno)": {hresult: 32 /*garbage*/, err: syscall.Errno(2) /*E_FILE_NOT_FOUND*/, wantError: true}, + "With a generic (unreachable) error": {hresult: 1, err: errors.New("test error"), wantError: true}, + } + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + t.Parallel() + res, err := microsoftstore.CheckError(tc.hresult, tc.err) + if tc.wantError { + require.Error(t, err, "CheckError should have returned an error for value: %v, returned value was: %v", tc.hresult, res) + } else { + require.NoError(t, err, "CheckError should have not returned an error for value: %v, returned value was: %v", tc.hresult, res) + } + }) + } +} + func buildStoreAPI(ctx context.Context) error { ctx, cancel := context.WithTimeout(ctx, 5*time.Minute) defer cancel() diff --git a/storeapi/go-wrapper/microsoftstore/store_windows.go b/storeapi/go-wrapper/microsoftstore/store_windows.go index 881fc79d3..2a79967d2 100644 --- a/storeapi/go-wrapper/microsoftstore/store_windows.go +++ b/storeapi/go-wrapper/microsoftstore/store_windows.go @@ -2,7 +2,6 @@ package microsoftstore import ( - "errors" "fmt" "sync" "syscall" @@ -93,29 +92,7 @@ func call(proc *syscall.LazyProc, args ...uintptr) (int64, error) { hresult, _, err := proc.Call(args...) //nolint:gosec // Windows HRESULTS are guaranteed to be 32-bit vlaue, thus they surely fit inside a int64 without overflow. - hres := int64(hresult) - // From syscall/dll_windows.go (*Proc).Call doc: - // > Callers must inspect the primary return value to decide whether an - // error occurred [...] before consulting the error. - if err := NewStoreAPIError(hres); err != nil { - return hres, fmt.Errorf("storeApi returned error code %d: %w", hres, err) - } - - if err == nil { - return hres, nil - } - - var target syscall.Errno - if b := errors.As(err, &target); !b { - // Supposedly unrechable: proc.Call must always return a syscall.Errno - return hres, err - } - - if target != syscall.Errno(0) { - return hres, fmt.Errorf("failed syscall to storeApi: %v (syscall errno %d)", target, err) - } - - return hres, nil + return checkError(int64(hresult), err) } // loadDll finds the dll and ensures it loads.