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

Unreachable log info of frame_try_runtime::TryRuntime #362

Closed
Chralt98 opened this issue May 23, 2022 Discussed in #361 · 3 comments
Closed

Unreachable log info of frame_try_runtime::TryRuntime #362

Chralt98 opened this issue May 23, 2022 Discussed in #361 · 3 comments

Comments

@Chralt98
Copy link

Discussed in #361

Originally posted by Chralt98 May 23, 2022
This will never get printed, if it relies on frame-executive. Because the first unwrap in try_runtime_upgrade() would lead to a panic, therefore the code stops executing and wouldn't reach the log::info from the call stack.

#![allow(unused)]
fn main() {
  try_runtime_upgrade().map_err(|err| {
      println!("Additional information");
      err
  }).unwrap();
}

fn try_runtime_upgrade() -> Result<(), &'static str> {
    let x: Result<(), &'static str> = Err("fatal error").unwrap();
    Ok(())
}
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "fatal error"', src/main.rs:7:58

"Additional information" never mentioned.

@Chralt98 Chralt98 changed the title Unreachable log info of frame_try_runtime::TryRuntime Unreachable log info of frame_try_runtime::TryRuntime May 23, 2022
@shawntabrizi
Copy link
Contributor

You shouldn't be using unwrap, you should be handling the result with something like ? or similar.

Of course if you unwrap and cause a panic, you will not trigger proper Rust error handling.

@Chralt98
Copy link
Author

My question: Isn't this code unreachable?

@weichweich
Copy link
Contributor

Yes that code is unreachable. I think at time of writing I didn't expect that Executive::try_runtime_upgrade() panics if the try runtime encountered an error.

weichweich added a commit that referenced this issue Jun 28, 2022
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

No branches or pull requests

3 participants