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

tracing::instrument on op_return_internal is broken #681

Closed
tgolsson opened this issue Mar 26, 2024 · 1 comment · Fixed by #682
Closed

tracing::instrument on op_return_internal is broken #681

tgolsson opened this issue Mar 26, 2024 · 1 comment · Fixed by #682
Labels
bug Something isn't working

Comments

@tgolsson
Copy link
Contributor

Noticed this when upping the log-level on an old project. If the return value doesn't have Protocol::DEBUG_STRING it'll just hard-crash when the return happens. Took quite a while to figure out why... :D

#[inline]
#[tracing::instrument(skip(self))]
fn op_return_internal(&mut self, return_value: Value) -> Result<bool, VmErrorKind> {
let exit = self.pop_call_frame()?;
self.stack.push(return_value)?;
Ok(exit)
}

I can point-fix it, but I think in general it'd be nicer if dbg! etc was less prone to exploding. Maybe an optional feature to "fall back" to something like <%T object at %p>, which isn't what most people would want but won't lead to spurious BOOM.

@udoprog udoprog added the bug Something isn't working label Mar 26, 2024
@udoprog
Copy link
Collaborator

udoprog commented Mar 26, 2024

Gaddangit! Well, I suppose we should add a fallible default debug impl than is used by default by Rust.

It should mostly be a matter of handling the error from the existing implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants