Skip to content

Commit

Permalink
Don't log routine errors as out-of-the-ordinary
Browse files Browse the repository at this point in the history
To benefit users in debug mode we log any unexpected exceptions to help
diagnose any issues that might arise. It turns out, though, we log this
for *every* exception happening for *every* import, including imports
like `__wbindgen_throw` which are explicitly intended to throw an
exception. This can cause distracting debug logs to get emitted to the
console, so let's squelch the debug logging for known imports that we
shouldn't log for, such as intrinsics.

Closes #1785
  • Loading branch information
alexcrichton committed Sep 25, 2019
1 parent 8b4fd2a commit 55dbf94
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 2 deletions.
13 changes: 11 additions & 2 deletions crates/cli-support/src/js/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ pub struct Builder<'a, 'b> {
/// Whether or not we're catching exceptions from the main function
/// invocation. Currently only used for imports.
catch: bool,
/// Whether or not we're logging the error coming out of this intrinsic
log_error: bool,
}

/// Helper struct used in incoming/outgoing to generate JS.
Expand All @@ -66,6 +68,7 @@ pub struct TypescriptArg {
impl<'a, 'b> Builder<'a, 'b> {
pub fn new(cx: &'a mut Context<'b>) -> Builder<'a, 'b> {
Builder {
log_error: cx.config.debug,
cx,
args_prelude: String::new(),
finally: String::new(),
Expand Down Expand Up @@ -98,6 +101,12 @@ impl<'a, 'b> Builder<'a, 'b> {
Ok(())
}

pub fn disable_log_error(&mut self, disable: bool) {
if disable {
self.log_error = false;
}
}

pub fn process(
&mut self,
binding: &Binding,
Expand All @@ -107,7 +116,7 @@ impl<'a, 'b> Builder<'a, 'b> {
invoke: &mut dyn FnMut(&mut Context, &mut String, &[String]) -> Result<String, Error>,
) -> Result<String, Error> {
// used in `finalize` below
if self.cx.config.debug {
if self.log_error {
self.cx.expose_log_error();
}

Expand Down Expand Up @@ -378,7 +387,7 @@ impl<'a, 'b> Builder<'a, 'b> {
// unhandled exceptions, typically used on imports. This currently just
// logs what happened, but keeps the exception being thrown to propagate
// elsewhere.
if self.cx.config.debug {
if self.log_error {
call = format!("try {{\n{}}} catch (e) {{\n logError(e)\n}}\n", call);
}

Expand Down
20 changes: 20 additions & 0 deletions crates/cli-support/src/js/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1937,6 +1937,7 @@ impl<'a> Context<'a> {
.unwrap();
self.export_function_table()?;
let mut builder = binding::Builder::new(self);
builder.disable_log_error(true);
let js = builder.process(&binding, &webidl, true, &None, &mut |_, _, args| {
Ok(format!(
"wasm.__wbg_function_table.get({})({})",
Expand Down Expand Up @@ -1965,6 +1966,7 @@ impl<'a> Context<'a> {
// Construct a JS shim builder, and configure it based on the kind of
// export that we're generating.
let mut builder = binding::Builder::new(self);
builder.disable_log_error(true);
match &export.kind {
AuxExportKind::Function(_) => {}
AuxExportKind::StaticFunction { .. } => {}
Expand Down Expand Up @@ -2058,8 +2060,10 @@ impl<'a> Context<'a> {
);
}

let disable_log_error = self.import_never_log_error(import);
let mut builder = binding::Builder::new(self);
builder.catch(catch)?;
builder.disable_log_error(disable_log_error);
let js = builder.process(
&binding,
&webidl,
Expand All @@ -2076,6 +2080,22 @@ impl<'a> Context<'a> {
}
}

/// Returns whether we should disable the logic, in debug mode, to catch an
/// error, log it, and rethrow it. This is only intended for user-defined
/// imports, not all imports of everything.
fn import_never_log_error(&self, import: &AuxImport) -> bool {
match import {
// Some intrinsics are intended to exactly throw errors, and in
// general we shouldn't have exceptions in our intrinsics to debug,
// so skip these.
AuxImport::Intrinsic(_) => true,

// Otherwise assume everything else gets a debug log of errors
// thrown in debug mode.
_ => false,
}
}

fn import_does_not_require_glue(
&self,
binding: &Binding,
Expand Down

0 comments on commit 55dbf94

Please sign in to comment.