Skip to content
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.

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

improve wasm error message #3781

Merged
merged 1 commit into from
Feb 6, 2023
Merged

Conversation

natasha41575
Copy link
Contributor

@natasha41575 natasha41575 commented Feb 2, 2023

Fix #3763

It looks like there is a flaky function in our wasm fnruntime library. Specifically this line is looking problematic:

https://github.com/GoogleContainerTools/kpt/blob/5a7e65fa4bf03f7948ec1365d4a4a0c30a59b889/internal/fnruntime/wasmtime.go#L145

It's supposed to populate result with the output ResourceList, but for me, maybe 1/20 times it is returning very odd-looking strings like s(<nil>) or s(float64=4.0474e-320. This currently results in kyaml throwing its very unhelpful Error: wrong Node Kind for expected: MappingNode was ScalarNode: value: {s(float64=5.565e-320} error.

This PR doesn't fix the flake, but it is trying to at least improve the error message to make it more obvious where it came from. I filed a separate issue for addressing the flake: #3782

Copy link
Contributor

@yuwenma yuwenma left a comment

Choose a reason for hiding this comment

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

That's fair. Thank you for the change!

@droot
Copy link
Contributor

droot commented Feb 2, 2023

maybe 1/20 times it is returning very odd-looking strings like

I am making a wild guess. Since wasm runtime is using js based bridge, it might be a symptom of that js bridge not ready yet when the function was invoked. So just a retry might help. You could try adding a retry (just once) around that function and see if retry succeed (if you have a reproducible setup).

Improving the error is anyway helpful.

@natasha41575
Copy link
Contributor Author

Thanks for the feedback! Looked like the user attempted a retry and is still running into the issue. I will merge this PR and we can use the other issue I filed for further tracking.

@natasha41575 natasha41575 merged commit 9dbfc60 into kptdev:main Feb 6, 2023
@natasha41575 natasha41575 deleted the wasmErr branch February 6, 2023 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WASM function error messages are unusable
3 participants