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

Remove RuntimeError::raise #2998

Closed
wants to merge 2 commits into from
Closed

Remove RuntimeError::raise #2998

wants to merge 2 commits into from

Conversation

fschutt
Copy link
Contributor

@fschutt fschutt commented Jul 1, 2022

Removes RuntimeError::raise (function was already depreceated for a relatively long time).

Review

  • Add a short description of the change to the CHANGELOG.md file

@fschutt fschutt requested a review from syrusakbary as a code owner July 1, 2022 12:50
@fschutt fschutt marked this pull request as draft July 1, 2022 12:50
Comment on lines -109 to -114
/// Raises a custom user Error
#[deprecated(since = "2.1.1", note = "return a Result from host functions instead")]
pub fn raise(error: Box<dyn Error + Send + Sync>) -> ! {
unsafe { raise_user_trap(error) }
}

Copy link
Member

Choose a reason for hiding this comment

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

This is good.

let error = Self::user(error);
let js_error: JsValue = error.into();
wasm_bindgen::throw_val(js_error)
}
Copy link
Member

Choose a reason for hiding this comment

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

Make it private

Ok(Ok(result)) => return result.into_c_struct(),
Ok(Err(trap)) => RuntimeError::raise(Box::new(trap)),
Ok(Ok(result)) => HostFunctionResult::Ok(result.into_c_struct()),
Ok(Err(trap)) => HostFunctionResult::Err(trap),
Copy link
Member

Choose a reason for hiding this comment

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

Keep this code as before

@@ -1000,6 +1000,14 @@ mod inner {
}
}

/// C-compatible `Result` with stable layout,
Copy link
Member

Choose a reason for hiding this comment

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

You don't need to change this file

@fschutt
Copy link
Contributor Author

fschutt commented Jul 4, 2022

Closed in favor of #3003

@fschutt fschutt closed this Jul 4, 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

Successfully merging this pull request may close these issues.

2 participants