Add inference error messages in WIT notebook mode #2414

jameswex · 2019-07-08T17:49:43Z

Motivation for features / changes

If a model fails to run inference in WitWidget, no error is currently displayed so the user has no idea why WIT isn't working correctly. This change adds in proper error handling.

Technical description of changes

Updated jupyter and colab witwidget code so that if inference throws any python error, it is properly caught and sent to the frontend.
The frontend now displays that inference error in a toast and in the status message.
Also fixed a small bug where when comparing 2 regression models, the initial Facets Dive view doesn't compare the two models.

Screenshots of UI changes

Detailed steps to verify changes work correctly (as executed by you)

Ran colab and jupyter notebooks with and without inference errors to see that the error message in both cases gets to the user.
Ran a two-model regression notebook to see proper default Dive display.

jameswex · 2019-07-08T17:50:06Z

@tolga-b please review

jameswex · 2019-07-09T15:16:12Z

@stephanwlee please review, thanks

stephanwlee · 2019-07-09T18:53:38Z

...ractive_inference/tf_interactive_inference_dashboard/tf-interactive-inference-dashboard.html

+      handleError_: function(errorStr) {
+        this.showToast_(errorStr);
+        this.exampleStatusStr = errorStr;
+        this.$.spinner.hidden = true;


Can you please use the Polymer data binding instead?

stephanwlee · 2019-07-09T18:55:08Z

...ractive_inference/tf_interactive_inference_dashboard/tf-interactive-inference-dashboard.html

-              this.exampleStatusStr = 'Request failed';
-              this.showToast_('Request failed: ' + reason);
-              this.$.spinner.hidden = true;
+              this.handleError_('Request failed: ' + reason);


Strings like 'Request failed' often don't mean anything to the user (especially if it is a toast; which request? TensorBoard's request?). Please be a little bit more descriptive.

good point. Adding new string param for what the request does, so that if it fails the error states what request failed (such as 'Datapoint load', or 'Model inference').

stephanwlee · 2019-07-09T19:00:36Z

tensorboard/plugins/interactive_inference/witwidget/notebook/colab/wit.py

+      output.eval_js("""inferenceCallback('{inferences}')""".format(
+        inferences=json.dumps(inferences)))
+    except Exception as e:
+      error_str = str(e)


How is this used?

thx, removed

stephanwlee · 2019-07-09T19:04:54Z

tensorboard/plugins/interactive_inference/witwidget/notebook/colab/wit.py

@@ -90,6 +90,9 @@ def infer_mutants(wit_id, details):

      // Javascript callbacks called by python code to communicate with WIT
      // Polymer element.
+      window.backendError = errorStr => {{
+        wit.handleError_(errorStr);


make handleError_ a public method

wchargin · 2019-07-09T20:00:15Z

tensorboard/plugins/interactive_inference/witwidget/notebook/colab/wit.py

-      inferences=json.dumps(inferences)))
+    try:
+      inferences = base.WitWidgetBase.infer_impl(self)
+      output.eval_js("""inferenceCallback('{inferences}')""".format(


Pardon the drive-by, but this is an injection vector when any of the
keys or values (recursively) of inferences contains a single quote:

>>> import json >>> inferences = {"label_vocab": "',alert('hmm'),null[0],'"} >>> js = """inferenceCallback('{inferences}')""".format( ... inferences=json.dumps(inferences)) >>> print(js) inferenceCallback('{"label_vocab": "',alert('hmm'),null[0],'"}')

I’d expect to see simply inferenceCallback({inferences}), without the
quotes, such that the JS gets an object literal rather than a string
literal. If it really needs to be a string for some reason, you can
create the string literal safely by JSON-encoding again (in either
Python or JS).

replaced with object and not string.

wchargin · 2019-07-09T20:04:13Z

tensorboard/plugins/interactive_inference/witwidget/notebook/colab/wit.py

+        inferences=json.dumps(inferences)))
+    except Exception as e:
+      error_str = str(e)
+      output.eval_js("""backendError('{error}')""".format(


Likewise an injection vector, and this one may be plausibly hit even in
a non-malicious case, because Python exception texts frequently contain
single quotes:

>>> import json >>> try: None.wat ... except Exception as e: ... js = """backendError('{error}')""".format(error=json.dumps(str(e))) ... print(js) ... backendError('"'NoneType' object has no attribute 'wat'"') >>> # ^ JS syntax error

(More occurrences below.)

replaced with object and not string.

jameswex · 2019-07-12T01:14:23Z

addressed @stephanwlee and @wchargin comments

stephanwlee

All comments are nits.

stephanwlee · 2019-07-12T15:03:40Z

...ractive_inference/tf_interactive_inference_dashboard/tf-interactive-inference-dashboard.html

      makeAsyncRequest_: function(
-          url, thenDoFn, postData, errorFn = () => {}) {
+          url, thenDoFn, postData, requestStr, errorFn = () => {}) {


super nit: requestStr -> humanReadableRequestName?

stephanwlee · 2019-07-12T15:05:33Z

...ractive_inference/tf_interactive_inference_dashboard/tf-interactive-inference-dashboard.html

-              this.exampleStatusStr = 'Request failed';
-              this.showToast_('Request failed: ' + reason);
-              this.$.spinner.hidden = true;
+              this.handleError(requestStr + ' failed: ' + reason);


`Request for ${humanReadableRequestName} failed: ${reason}`

I thought it would be confusing to show a toast that says something like Model inference failed: foobarbaz

stephanwlee · 2019-07-12T15:11:30Z

tensorboard/plugins/interactive_inference/witwidget/notebook/colab/wit.py

+    try:
+      inferences = base.WitWidgetBase.infer_impl(self)
+      output.eval_js("""inferenceCallback({inferences})""".format(
+        inferences=json.dumps(inferences)))


as per http://google.github.io/styleguide/pyguide.html#3192-line-breaking, you might want +2 more whitespace here and below.

Requests addressed; thank you!

jameswex · 2019-07-15T14:57:10Z

@stephanwlee addressed your comments, thanks

add error messages

fbfcf69

jameswex added the plugin:what-if-tool label Jul 8, 2019

fixed up jupyter duplicate error issue

8d98186

googlebot added the cla: yes label Jul 9, 2019

tolga-b approved these changes Jul 9, 2019

View reviewed changes

jameswex requested a review from stephanwlee July 9, 2019 15:16

stephanwlee reviewed Jul 9, 2019

View reviewed changes

wchargin previously requested changes Jul 9, 2019

View reviewed changes

jameswex added 2 commits July 11, 2019 11:02

review comments part 1

c37d2b0

review comments 2

02cb9ae

stephanwlee self-requested a review July 12, 2019 14:55

stephanwlee reviewed Jul 12, 2019

View reviewed changes

review comments

c69882f

stephanwlee approved these changes Jul 15, 2019

View reviewed changes

jameswex merged commit 839ceac into tensorflow:master Jul 15, 2019

jameswex deleted the wit_display_error branch July 15, 2019 16:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add inference error messages in WIT notebook mode #2414

Add inference error messages in WIT notebook mode #2414

jameswex commented Jul 8, 2019

jameswex commented Jul 8, 2019

jameswex commented Jul 9, 2019

stephanwlee Jul 9, 2019

jameswex Jul 11, 2019

stephanwlee Jul 9, 2019

jameswex Jul 11, 2019

stephanwlee Jul 9, 2019

jameswex Jul 11, 2019

stephanwlee Jul 9, 2019

jameswex Jul 11, 2019

wchargin Jul 9, 2019

jameswex Jul 12, 2019

wchargin Jul 9, 2019 •

edited

Loading

jameswex Jul 12, 2019

jameswex commented Jul 12, 2019

stephanwlee left a comment

stephanwlee Jul 12, 2019

jameswex Jul 15, 2019

stephanwlee Jul 12, 2019

jameswex Jul 15, 2019

stephanwlee Jul 12, 2019

jameswex Jul 15, 2019

jameswex commented Jul 15, 2019

Add inference error messages in WIT notebook mode #2414

Add inference error messages in WIT notebook mode #2414

Conversation

jameswex commented Jul 8, 2019

jameswex commented Jul 8, 2019

jameswex commented Jul 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wchargin Jul 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jameswex commented Jul 12, 2019

stephanwlee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jameswex commented Jul 15, 2019

wchargin Jul 9, 2019 •

edited

Loading