From 55dbf9478ffedacb5c58f24a92bbbce19a99a3b6 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Tue, 24 Sep 2019 07:59:02 -0700
Subject: [PATCH] Don't log routine errors as out-of-the-ordinary

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
---
 crates/cli-support/src/js/binding.rs | 13 +++++++++++--
 crates/cli-support/src/js/mod.rs     | 20 ++++++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/crates/cli-support/src/js/binding.rs b/crates/cli-support/src/js/binding.rs
index 0563b0a4..34d417e6 100644
--- a/crates/cli-support/src/js/binding.rs
+++ b/crates/cli-support/src/js/binding.rs
@@ -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.
@@ -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(),
@@ -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,
@@ -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();
         }
 
@@ -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);
         }
 
diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs
index 6a7bba55..5364decc 100644
--- a/crates/cli-support/src/js/mod.rs
+++ b/crates/cli-support/src/js/mod.rs
@@ -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({})({})",
@@ -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 { .. } => {}
@@ -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,
@@ -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,