[Reporting] Saved POST URLs for dashboard are not migration-capable

[tsullivan]

Kibana version: This affects migrations:

• 7.12 -> 7.13
• 7.13 -> 7.14

It's possible to create a scheduled report (using a POST URL in Watcher or a shell script) using Kibana 7.13 and get a bug after upgrading to Kibana 7.14 or above. This is because the POST URL string stores dashboard state that might only work in 7.13.

Steps:

1. Using Kibana 7.13, create a dashboard that has a Lens By-Value panel
2. Save the dashboard
3. Copy the POST URL and create a shell script to test it:

   #!/bin/sh
   curl -u my_reporting_user -H "kbn-version: ${KIBANA_VERSION}" -XPOST "<enter the POST URL between double quotes>"
   

   Save as test_post_url.sh, run chmod a+x test_post_url.sh, then run:

   KIBANA_VERSION=7.13.2 ./test_post_url.sh
   

4. Upgrade Kibana and Elasticsearch to 7.14, making sure to preserve all the data
5. Run the test script again:

   KIBANA_VERSION=7.14.0 ./test_post_url.sh
   

The job will fail due to a TypeError on the page when the report job executes.

Notes:

1. Saving the dashboard before PDF/PNG export is required, but the job params contain the page URL as sent in ShareContext.shareableUrl which includes the state of the dashboard: it is not just a link to the dashboard by ID. This is also referred to as a "snapshot URL"
2. You can inspect jobParams data in the POST URL by decoding the URI-encoded characters, and converting the string of RISON data into JSON
   • URI decoder: https://meyerweb.com/eric/tools/dencoder/
   • Rison decoder: https://observablehq.com/@nyurik/rison-online-decoder-encoder

cc @stacey-gammon @ThomThomson @jloleysens @streamich @ppisljar

cc @elastic/kibana-reporting-services

[Petr] Testing sync of bug label....

[elasticmachine]

Pinging @elastic/kibana-app-services (Team:AppServices)

[elasticmachine]

Pinging @elastic/kibana-app (Team:KibanaApp)

[elasticmachine]

Pinging @elastic/kibana-reporting-services (Team:Reporting Services)

[flash1293]

This seems like an issue with how dashboard is integrating with the share plugin - how can the Kibana app help here?

[ppisljar]

@joe most likely we'll resolve this internally. Once new URL service gets introduced reporting will no longer store the URL but LocatorState which implements PersistableState interface, meaning it supports migrations and those will be handled internally (when using LocatorState it will be automatically migrated to latest version)

[tsullivan]

@flash1293 as Peter mentioned, we have already-planned work to add a new export type that does not store the URL but will use a versioned state object that can be converted into a deep-link URL at runtime. We planned on doing this as a solution towards not requiring the dashboard to be saved before creating a report. It turns out to be a convenient answer to preventing issues like this one with non-versioned Lens state being in the URLs.

To really close this issue, we need to observe the impact of this bug. If it gives no clear reason for a report failing other than "time out" then we need to fix that so the user sees information that guides them towards fixing the POST URL they have saved for automation and set up in an older version.

[flash1293]

To really close this issue, we need to observe the impact of this bug. If it gives no clear reason for a report failing other than "time out" then we need to fix that so the user sees information that guides them towards fixing the POST URL they have saved for automation and set up in an older version.

@tsullivan I just tested and this is how it fails:

We could add a generic try catch in the right place, rewrite the error to say something generic like Could not render chart, unexpected configuration, but I would like to avoid something reporting specific in the wording because it could theoretically show up in other places as well.

[tsullivan]

We could add a generic try catch in the right place, rewrite the error to say something generic like Could not render chart, unexpected configuration, but I would like to avoid something reporting specific in the wording because it could theoretically show up in other places as well.

@flash1293 could the app tell based on the URL having unversioned state that rendering will be a problem? If the app can detect an upcoming failure, it could signal that to Reporting by adding error text in an agreed-upon data attribute in a DOM element. Data attributes in the app have always been used to "talk" to Reporting.

1. Dashboard finds unversioned persisted app state in the URL.
2. It renders only the panels it is capable of, given the state it was able to extract
3. Since some panels could not be rendered, Dashboard write an error message into a DOM element:

   $(selectorOfPanelWithErrors).attr({
     "data-render-error": `The link to this dashboard must be regenerated!`,
     "data-render-complete": true,
   });

Reporting will select for elements that have the data-render-error attribute and know to set an error on the job that will stand out to the user.

[flash1293]

I looked into the code and it seems like a panel can be turned into a proper "ErrorEmbeddable" by emitting an error in the output observable of the embeddable instance. The Lens embeddable could catch the error happening and do exactly that - the dashboard should handle it from there.

kibana/src/plugins/embeddable/public/lib/panel/embeddable_panel.tsx

Line 297 in dfc70bd

const errorEmbeddable = new ErrorEmbeddable(error, { id: this.props.embeddable.id });

@ThomThomson Does this sound like a good approach? If yes I can open an issue for that, it's something that would be good to have even unrelated to this specific bug.

Edit: Actually I'm not sure whether the dashboard will set data-render-error correctly for error embeddables, but it probably should.

[ppisljar]

another thing to note: if dashboard would be doing client side migrations this wouldn't be an issue cc @ThomThomson

[ThomThomson]

@flash1293, the more errors that are caught correctly the better, and I can look into implementing data-render-error on the error embeddables to expose the error message.

@ppisljar, I agree that once we figure out persistable URLs - whether the solution is clientside migrations or not - this issue will disappear. In the meantime, better error reporting seems like a good direction to take this.

[jloleysens]

@ThomThomson and I met to chat about this issue. Here are some of the important points from our discussion:

• The un-migrated state issue is only present in POST URLs containing Lens by-value panels - this is a relatively small subset of URLs out there. We should be careful about taking action that might affect all POST URLs that are still working OK.
• Related to 👆🏻, we need to be careful about presenting error messages that ask users to regenerate unversioned URLs as the fix. We cannot guarantee regenerating will solve the underlying issue (there might be another bug unrelated to state migration). Regenerating a URL and still seeing an issue would be a frustrating UX we should try to avoid.
• We need to be surfacing better error messages on Reports in general. This is likely a cross-team effort that would require auditing any visual that might break and ensuring an appropriate, user-facing message is surfaced (where reasonably possible).
• Reporting does not know anything about what it is rendering (currently untrue, but let's assume that for now). There might be 0-n issues on a given page. That does not mean we should fail the report, it just means we need a way to handle many data-render-warnings in a sensible way that keeps users informed/empowered.
• For reporting we are working on a way for app state to be passed via history.location.state rather than the URL (see [Reporting] Create reports with full state required to generate the report #101048) - currently targeting 7.15.0 which avoids issues with un-migrated URLs. Plugins using reporting will need a locator implementation that handles the app-specific migrations, Reporting will call these before navigating to the plugin/visual thus passing in migrated state.

Given all of the above I think the following steps could be taken:

1. Reporting creates a new mechanism for plugins to surface warning messages data-render-warning (rather than errors). Here we could recommend that users regenerate their URL to avoid potential issues.
2. Reporting will surface these messages in logs and in the UI. AFAICT, this requires a bit of UI/UX design in the Reporting management UI.
3. In the case of dashboards, unversioned URLs could be detected and result in a data-render-warning asking users to regenerate the Report URL (assuming we will have versioned URLs)
4. Plugins should migrate to the new export types once they are ready (old PDF/PNGs will be legacy)
5. (the potential biggest task) Audit error messages we surface from visualisations and map these to data-render-warning as user facing copy.

Please keep honest about any of the above @ThomThomson and @tsullivan

[ThomThomson]

Some clarification for potential task #5: There is already a user-facing copy generation step which should be happening when the embeddables surface errors. ErrorEmbeddables are already a user-facing construct, so if the EmbeddableContainer was changed to surface ErrorEmbeddable errors as data-render-warning the only extra work required would be to capture & display those errors in the reporting UI.

[tsullivan]

Related issue: #97904 Reporting fails the report if there is a missing saved object referenced in a dashboard panel. I think our conversation going on in this issue about data-render-warning will cover these cases as well.

[flash1293]

removing vis editors label as the embeddable error reporting part is split out in a separate issue.

[jloleysens]

After merging:

• [Reporting] Update chromium exit behaviour #113544 (Chromium exit behaviour)
• [Reporting] Add new data-render-error attribute #114472 (new data-render-error attribute)

We should be in a good position to help users out when their Reports are not generating as they expect, for whatever reason. The primary purpose of this issue was to explore how to deal with un-migratable state in reporting URLs which is something we addressed moving forward by adding a locator which should migrate state (#108553).

For old URLs that may be running into this issue, users should now get a message from reporting stating that, for e.g., 3/4 visualizations are not rendering. There is follow up work in Lens to provide better messaging (see #104395) where old state can be called out.

Anyone please feel free to re-open if they feel differently!