Skip to content

Fix error message #1603

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix error message #1603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions pkg/workflows/wasm/host/internal/rawsdk/helpers_wasip1.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,12 @@ func await[I, O proto.Message](input I, output O, fn awaitFn) {
bytes := fn(mptr, mlen, responsePtr, responseLen)

if bytes < 0 {
SendError(errors.New("awaitCapabilities returned an error"))
SendError(fmt.Errorf("awaitCapabilities return an error: %s", response[:-bytes]))
}

if proto.Unmarshal(response[:bytes], output) != nil {
SendError(errors.New("failed to proto unmarshal await response"))
err := proto.Unmarshal(response[:bytes], output)
if err != nil {
SendError(fmt.Errorf("failed to unmarshal await response: %w", err))
}
}

Expand Down
18 changes: 13 additions & 5 deletions pkg/workflows/wasm/host/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,14 @@ func truncateWasmWrite(caller *wasmtime.Caller, src []byte, ptr int32, size int3
return write(memory, src, ptr, size)
}

func truncateWasmWriteErr(caller *wasmtime.Caller, src []byte, ptr int32, size int32) int64 {
code := truncateWasmWrite(caller, src, ptr, size)
if code > 0 {
return -int64(len(src))
}
return code
}

// write copies the given src byte slice into the memory at the given pointer and max size.
func write(memory, src []byte, ptr, maxSize int32) int64 {
if ptr < 0 {
Expand Down Expand Up @@ -1145,36 +1153,36 @@ func createAwaitSecretsFn(
if err != nil {
errStr := fmt.Sprintf("error reading from wasm %s", err)
logger.Error(errStr)
return truncateWasmWrite(caller, []byte(errStr), responseBuffer, maxResponseLen)
return truncateWasmWriteErr(caller, []byte(errStr), responseBuffer, maxResponseLen)
}

req := &sdkpb.AwaitSecretsRequest{}
err = proto.Unmarshal(b, req)
if err != nil {
errStr := err.Error()
logger.Error(errStr)
return truncateWasmWrite(caller, []byte(errStr), responseBuffer, maxResponseLen)
return truncateWasmWriteErr(caller, []byte(errStr), responseBuffer, maxResponseLen)
}

resp, err := exec.awaitSecrets(exec.ctx, req)
if err != nil {
errStr := err.Error()
logger.Error(errStr)
return truncateWasmWrite(caller, []byte(errStr), responseBuffer, maxResponseLen)
return truncateWasmWriteErr(caller, []byte(errStr), responseBuffer, maxResponseLen)
}

respBytes, err := proto.Marshal(resp)
if err != nil {
errStr := err.Error()
logger.Error(errStr)
return truncateWasmWrite(caller, []byte(errStr), responseBuffer, maxResponseLen)
return truncateWasmWriteErr(caller, []byte(errStr), responseBuffer, maxResponseLen)
}

size := wasmWrite(caller, respBytes, responseBuffer, maxResponseLen)
if size == -1 {
errStr := ResponseBufferTooSmall
logger.Error(errStr)
return truncateWasmWrite(caller, []byte(errStr), responseBuffer, maxResponseLen)
return truncateWasmWriteErr(caller, []byte(errStr), responseBuffer, maxResponseLen)
}

return size
Expand Down
14 changes: 14 additions & 0 deletions pkg/workflows/wasm/host/standard_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,20 @@ func TestStandardSecrets(t *testing.T) {
require.Equal(t, "Bar", result.GetValue().GetStringValue())
})

t.Run("returns an error when there are invalid UTF-8 bytes", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add one that shows the SDK returns the right error from the host?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests fail because you didn't implement the STD test for common. This version of it verifies the test can pass. Once you have that, I would implement it in Go's SDK too to ensure assumptions you made are correct about messaging (eg the bar question that Street asked below).

It sucks, but it keeps us from having a circular dependency. I think of Go as the reference implementation, even though it's not with the host, it's good to ensure it passes.

b := []byte{0xff, 0xfe, 0xfd} // invalid UTF-8 bytes
s := string(b)

result := runSecretTest(t, m, &sdk.SecretResponse{
Response: &sdk.SecretResponse_Secret{
Secret: &sdk.Secret{
Value: s,
},
},
})
require.Equal(t, "Bar", result.GetValue().GetStringValue(), result.GetError())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe missing some parent test context, but where does the value "Bar" come from if this is expected to error?

})

t.Run("returns an error if the secret doesn't exist", func(t *testing.T) {
resp := runSecretTest(t, m, &sdk.SecretResponse{
Response: &sdk.SecretResponse_Error{
Expand Down
Loading