Plotly.js: On-figure error for mapbox-token-related problems

[nicolaskruchten]

For the two token-related errors, we should show the error in the figure:

• no token, no style
• mapbox style, no token

There's a third we could try to catch:

• mapbox style, mapbox token: invalid token

[etpinard]

(Writing down a few thoughts from a private convo with @nicolaskruchten )

Instead of showing the errors on the figure (like we currently do for WebGL-initialisation errors), perhaps it would be better to add a way to propagate ALL Lib.log, Lib.warn and Lib.error messages out of the JS console and into the graph div behind some new config flag (e.g. named notifyLogs). I'm thinking of reusing the Lib.notifier we currently use for e.g. double-click tips to show those messages.

[nicolaskruchten]

This would be great for the “totals don’t match” warnings from sunburst/treemap as well for Python/R users who won’t naturally go check the console or might not be able to easily (ie in RStudio)

[alexcjohnson]

That sounds great, except that Lib.notifier goes away by itself fairly quickly. These errors should stick around until the user clears them. Hopefully a small tweak to make notifier support that mode.

[etpinard]

Here's a WIP branch that's add a new config option that allows log/warn/error messages to show up in "notifier" on-graph popups

https://github.com/plotly/plotly.js/compare/onGraphLogging

Demos:

• branchvalues total problems: https://codepen.io/etpinard/pen/jOEGqdL
• mapbox style, no token: https://codepen.io/etpinard/pen/oNgGxMV
• no token, no style: https://codepen.io/etpinard/pen/qBEPZyL

Let me know what you think.

[archmoj]

Nicely done. I'd vote for reducing the transition time a bit.

[etpinard]

Ping @nicolaskruchten

[nicolaskruchten]

Looks awesome to me! I wonder about the name onGraphLogging though... it's not really on graph any more? how about something like notifyOnWarnings or something?

[etpinard]

onGraphLogging comes from the current logging config option, but yeah we could do better. I'd be ok with notifyOnWarnings if you prefer.

[nicolaskruchten]

Semi-related: what is the current "level" for hierarchy-related issues in sunburst and treemap? Right now they don't show up in the Dash "debug" mode developer tools because they're not JS errors being thrown I think. The Mapbox-token-related stuff does show up though. It would be really helpful for Dash users to have the sunburst/treemap stuff appear in the Dash dev tools.

[etpinard]

what is the current "level" for hierarchy-related issues in sunburst and treemap?

They use Lib.warn, so they should show up in the console under logging: 1 or logging: 2.

[etpinard]

Maybe dash only catches the throw new Error() ?

[nicolaskruchten]

Yeah it does. Is there a way to catch the log messages from a wrapper?

[nicolaskruchten]

Or could we consider making the hierarchy stuff actual errors? they seem to be at the same level of severity as mapbox token stuff to me...

[etpinard]

Or could we consider making the hierarchy stuff actual errors?

We could, but to me mapbox here is the exception.

At present, we don't throw errors on bad data/layout inputs. We only throw errors on bad API calls (e.g. when you call Plotly.newPlot(gd) with gd not being a <div>) and on bad config input (e.g. when setting modeBarButtonsToAdd) but never on data/layout inputs. Now mapbox access tokens can be set as a config option OR a layout attribute, so there's a bit of an inconsistency there, but still.

Yeah it does. Is there a way to catch the log messages from a wrapper?

Not that I can think of, but it might be cool to trigger events e.g. plotly_log, plotly_warn, plotly_error.

[nicolaskruchten]

Works for me: https://github.com/plotly/plotly.js/issues/4462