Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Reporting] Update chromium exit behaviour #113544

Conversation

jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Sep 30, 2021

Summary

The following issues report poor UX associated with reporting crashing on unhandled page exceptions:

The result is that users are not empowered to dig deeper into resolving the issue (in both cases it is a bad URL, one has a bad ID reference and one contains outdated, incompatible state for dashboard).

The changes proposed here update the chromium's page exit behaviour to not close when an uncaught exceptions occurs on the page. Specifically, we don't close the browser on the pageerror event. This is one step closer to improving errors for reporting.

The effect of this change is that for URLs visited during the report generation process, we will still generate a screenshot and render any warnings that may occur.

Case: URLs with out-of-date state

With throwing behaviour (current)

  • Errored report generation attempt
  • No screenshot captured
Screenshots Screenshot 2021-09-30 at 14 39 57 Screenshot 2021-09-30 at 14 40 12

Logging exception, but not exiting (proposed changes)

  • Report is generated, with warnings and explanation that 3/4 visualisations loaded
  • Screenshots is captured (just of what Chromium can see)
Screenshots Screenshot 2021-09-30 at 14 42 18 Screenshot 2021-09-30 at 14 42 28 Screenshot 2021-09-30 at 14 42 37
Sample error log
server    log   [12:35:39.565] [warning][PNG][browser-driver][execute][kudy4mbq0duz9d0062c2fizp][plugins][png][reporting][runTask] Reporting encountered an uncaught error on the page that will be ignored. Error: TypeError: Cannot read property 'input' of undefined
    at http://localhost:5601/iyn/9007199254740991/bundles/plugin/lens/8.0.0/lens.chunk.3.js:35670:199
    at Array.filter (<anonymous>)
    at getErrorMessages (http://localhost:5601/iyn/9007199254740991/bundles/plugin/lens/8.0.0/lens.chunk.3.js:35670:44)
    at http://localhost:5601/iyn/9007199254740991/bundles/plugin/lens/8.0.0/lens.chunk.3.js:24042:107
    at Array.map (<anonymous>)
    at Object.getErrorMessages (http://localhost:5601/iyn/9007199254740991/bundles/plugin/lens/8.0.0/lens.chunk.3.js:24039:56)
    at validateDatasourceAndVisualization (http://localhost:5601/iyn/9007199254740991/bundles/plugin/lens/8.0.0/lens.chunk.0.js:18762:151)
    at persistedStateToExpression (http://localhost:5601/iyn/9007199254740991/bundles/plugin/lens/8.0.0/lens.chunk.0.js:18704:28)
    at async Object.documentToExpression (http://localhost:5601/iyn/9007199254740991/bundles/plugin/lens/8.0.0/lens.chunk.3.js:16175:14)
    at async Embeddable.initializeSavedVis (http://localhost:5601/iyn/9007199254740991/bundles/plugin/lens/8.0.0/lens.chunk.3.js:16496:9)

Checklist

In this case, if we did not throw on the page error we have a better chance of guiding the user back to dashboard, where the actual problem is that needs to be addressed.

How to test

  1. Start Kibana & ES
  2. Copy the curl command provided below (taken from a 7.13.0 instance) and run it against Kibana
  3. Run the script
  4. Go to the Reporting Management UI and inspect the logs
# 1. Copy this to a file and "chmod a+x ./request.sh" it
# 2. Update <prefix> to your local, dev value

#! /bin/sh

curl\
	-XPOST\
	-u elastic:changeme\
	-H 'kbn-xsrf: aaa'\
	http://localhost:5601/<prefix>/api/reporting/generate/png?jobParams=%28browserTimezone%3AEurope%2FAmsterdam%2Clayout%3A%28dimensions%3A%28height%3A848%2Cwidth%3A2560%29%2Cid%3Apng%29%2CobjectType%3Adashboard%2CrelativeUrl%3A%27%2Fapp%2Fdashboards%23%2Fview%2Febd0dd80-2135-11ec-ba28-234eedb64ef3%3F_g%3D%28filters%3A%21%21%28%29%2CrefreshInterval%3A%28pause%3A%21%21t%2Cvalue%3A0%29%2Ctime%3A%28from%3Anow-15m%2Cto%3Anow%29%29%26_a%3D%28description%3A%21%27%21%27%2Cfilters%3A%21%21%28%29%2CfullScreenMode%3A%21%21f%2Coptions%3A%28hidePanelTitles%3A%21%21f%2CuseMargins%3A%21%21t%29%2Cpanels%3A%21%21%28%28embeddableConfig%3A%28enhancements%3A%28%29%2CsavedVis%3A%28data%3A%28aggs%3A%21%21%28%29%2CsearchSource%3A%28%29%29%2Cdescription%3A%21%27%21%27%2Cparams%3A%28spec%3A%21%27%257B%250A%252F%2A%250A%250AWelcome%2520to%2520Vega%2520visualizations.%2520%2520Here%2520you%2520can%2520design%2520your%2520own%2520dataviz%2520from%2520scratch%2520using%2520a%2520declarative%2520language%2520called%2520Vega%2C%2520or%2520its%2520simpler%2520form%2520Vega-Lite.%2520%2520In%2520Vega%2C%2520you%2520have%2520the%2520full%2520control%2520of%2520what%2520data%2520is%2520loaded%2C%2520even%2520from%2520multiple%2520sources%2C%2520how%2520that%2520data%2520is%2520transformed%2C%2520and%2520what%2520visual%2520elements%2520are%2520used%2520to%2520show%2520it.%2520%2520Use%2520help%2520icon%2520to%2520view%2520Vega%2520examples%2C%2520tutorials%2C%2520and%2520other%2520docs.%2520%2520Use%2520the%2520wrench%2520icon%2520to%2520reformat%2520this%2520text%2C%2520or%2520to%2520remove%2520comments.%250A%250AThis%2520example%2520graph%2520shows%2520the%2520document%2520count%2520in%2520all%2520indexes%2520in%2520the%2520current%2520time%2520range.%2520%2520You%2520might%2520need%2520to%2520adjust%2520the%2520time%2520filter%2520in%2520the%2520upper%2520right%2520corner.%250A%2A%252F%250A%250A%2520%2520%24schema%3A%2520https%3A%252F%252Fvega.github.io%252Fschema%252Fvega-lite%252Fv5.json%250A%2520%2520title%3A%2520Event%2520counts%2520from%2520all%2520indexes%250A%250A%2520%2520%252F%252F%2520Define%2520the%2520data%2520source%250A%2520%2520data%3A%2520%257B%250A%2520%2520%2520%2520url%3A%2520%257B%250A%252F%2A%250AAn%2520object%2520instead%2520of%2520a%2520string%2520for%2520the%2520%2522url%2522%2520param%2520is%2520treated%2520as%2520an%2520Elasticsearch%2520query.%2520Anything%2520inside%2520this%2520object%2520is%2520not%2520part%2520of%2520the%2520Vega%2520language%2C%2520but%2520only%2520understood%2520by%2520Kibana%2520and%2520Elasticsearch%2520server.%2520This%2520query%2520counts%2520the%2520number%2520of%2520documents%2520per%2520time%2520interval%2C%2520assuming%2520you%2520have%2520a%2520%40timestamp%2520field%2520in%2520your%2520data.%250A%250AKibana%2520has%2520a%2520special%2520handling%2520for%2520the%2520fields%2520surrounded%2520by%2520%2522%2525%2522.%2520%2520They%2520are%2520processed%2520before%2520the%2520the%2520query%2520is%2520sent%2520to%2520Elasticsearch.%2520This%2520way%2520the%2520query%2520becomes%2520context%2520aware%2C%2520and%2520can%2520use%2520the%2520time%2520range%2520and%2520the%2520dashboard%2520filters.%250A%2A%252F%250A%250A%2520%2520%2520%2520%2520%2520%252F%252F%2520Apply%2520dashboard%2520context%2520filters%2520when%2520set%250A%2520%2520%2520%2520%2520%2520%2525context%2525%3A%2520true%250A%2520%2520%2520%2520%2520%2520%252F%252F%2520Filter%2520the%2520time%2520picker%2520%28upper%2520right%2520corner%29%2520with%2520this%2520field%250A%2520%2520%2520%2520%2520%2520%2525timefield%2525%3A%2520%40timestamp%250A%250A%252F%2A%250ASee%2520.search%28%29%2520documentation%2520for%2520%3A%2520%2520https%3A%252F%252Fwww.elastic.co%252Fguide%252Fen%252Felasticsearch%252Fclient%252Fjavascript-api%252Fcurrent%252Fapi-reference.html%2523api-search%250A%2A%252F%250A%250A%2520%2520%2520%2520%2520%2520%252F%252F%2520Which%2520index%2520to%2520search%250A%2520%2520%2520%2520%2520%2520index%3A%2520_all%250A%2520%2520%2520%2520%2520%2520%252F%252F%2520Aggregate%2520data%2520by%2520the%2520time%2520field%2520into%2520time%2520buckets%2C%2520counting%2520the%2520number%2520of%2520documents%2520in%2520each%2520bucket.%250A%2520%2520%2520%2520%2520%2520body%3A%2520%257B%250A%2520%2520%2520%2520%2520%2520%2520%2520aggs%3A%2520%257B%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520time_buckets%3A%2520%257B%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520date_histogram%3A%2520%257B%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%252F%252F%2520Use%2520date%2520histogram%2520aggregation%2520on%2520%40timestamp%2520field%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520field%3A%2520%40timestamp%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%252F%252F%2520The%2520interval%2520value%2520will%2520depend%2520on%2520the%2520daterange%2520picker%2520%28true%29%2C%2520or%2520use%2520an%2520integer%2520to%2520set%2520an%2520approximate%2520bucket%2520count%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520interval%3A%2520%257B%2525autointerval%2525%3A%2520true%257D%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%252F%252F%2520Make%2520sure%2520we%2520get%2520an%2520entire%2520range%2C%2520even%2520if%2520it%2520has%2520no%2520data%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520extended_bounds%3A%2520%257B%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%252F%252F%2520Use%2520the%2520current%2520time%2520range%21%21%21%27s%2520start%2520and%2520end%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520min%3A%2520%257B%2525timefilter%2525%3A%2520%2522min%2522%257D%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520max%3A%2520%257B%2525timefilter%2525%3A%2520%2522max%2522%257D%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%257D%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%252F%252F%2520Use%2520this%2520for%2520linear%2520%28e.g.%2520line%2C%2520area%29%2520graphs.%2520%2520Without%2520it%2C%2520empty%2520buckets%2520will%2520not%2520show%2520up%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520min_doc_count%3A%25200%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%257D%250A%2520%2520%2520%2520%2520%2520%2520%2520%2520%2520%257D%250A%2520%2520%2520%2520%2520%2520%2520%2520%257D%250A%2520%2520%2520%2520%2520%2520%2520%2520%252F%252F%2520Speed%2520up%2520the%2520response%2520by%2520only%2520including%2520aggregation%2520results%250A%2520%2520%2520%2520%2520%2520%2520%2520size%3A%25200%250A%2520%2520%2520%2520%2520%2520%257D%250A%2520%2520%2520%2520%257D%250A%252F%2A%250AElasticsearch%2520will%2520return%2520results%2520in%2520this%2520format%3A%250A%250Aaggregations%3A%2520%257B%250A%2520%2520time_buckets%3A%2520%257B%250A%2520%2520%2520%2520buckets%3A%2520%255B%250A%2520%2520%2520%2520%2520%2520%257B%250A%2520%2520%2520%2520%2520%2520%2520%2520key_as_string%3A%25202015-11-30T22%3A00%3A00.000Z%250A%2520%2520%2520%2520%2520%2520%2520%2520key%3A%25201448920800000%250A%2520%2520%2520%2520%2520%2520%2520%2520doc_count%3A%25200%250A%2520%2520%2520%2520%2520%2520%257D%2C%250A%2520%2520%2520%2520%2520%2520%257B%250A%2520%2520%2520%2520%2520%2520%2520%2520key_as_string%3A%25202015-11-30T23%3A00%3A00.000Z%250A%2520%2520%2520%2520%2520%2520%2520%2520key%3A%25201448924400000%250A%2520%2520%2520%2520%2520%2520%2520%2520doc_count%3A%25200%250A%2520%2520%2520%2520%2520%2520%257D%250A%2520%2520%2520%2520%2520%2520...%250A%2520%2520%2520%2520%255D%250A%2520%2520%257D%250A%257D%250A%250AFor%2520our%2520graph%2C%2520we%2520only%2520need%2520the%2520list%2520of%2520bucket%2520values.%2520%2520Use%2520the%2520format.property%2520to%2520discard%2520everything%2520else.%250A%2A%252F%250A%2520%2520%2520%2520format%3A%2520%257Bproperty%3A%2520%2522aggregations.time_buckets.buckets%2522%257D%250A%2520%2520%257D%250A%250A%2520%2520%252F%252F%2520%2522mark%2522%2520is%2520the%2520graphics%2520element%2520used%2520to%2520show%2520our%2520data.%2520%2520Other%2520mark%2520values%2520are%3A%2520area%2C%2520bar%2C%2520circle%2C%2520line%2C%2520point%2C%2520rect%2C%2520rule%2C%2520square%2C%2520text%2C%2520and%2520tick.%2520%2520See%2520https%3A%252F%252Fvega.github.io%252Fvega-lite%252Fdocs%252Fmark.html%250A%2520%2520mark%3A%2520line%250A%250A%2520%2520%252F%252F%2520%2522encoding%2522%2520tells%2520the%2520%2522mark%2522%2520what%2520data%2520to%2520use%2520and%2520in%2520what%2520way.%2520%2520See%2520https%3A%252F%252Fvega.github.io%252Fvega-lite%252Fdocs%252Fencoding.html%250A%2520%2520encoding%3A%2520%257B%250A%2520%2520%2520%2520x%3A%2520%257B%250A%2520%2520%2520%2520%2520%2520%252F%252F%2520The%2520%2522key%2522%2520value%2520is%2520the%2520timestamp%2520in%2520milliseconds.%2520%2520Use%2520it%2520for%2520X%2520axis.%250A%2520%2520%2520%2520%2520%2520field%3A%2520key%250A%2520%2520%2520%2520%2520%2520type%3A%2520temporal%250A%2520%2520%2520%2520%2520%2520axis%3A%2520%257Btitle%3A%2520false%257D%2520%252F%252F%2520Customize%2520X%2520axis%2520format%250A%2520%2520%2520%2520%257D%250A%2520%2520%2520%2520y%3A%2520%257B%250A%2520%2520%2520%2520%2520%2520%252F%252F%2520The%2520%2522doc_count%2522%2520is%2520the%2520count%2520per%2520bucket.%2520%2520Use%2520it%2520for%2520Y%2520axis.%250A%2520%2520%2520%2520%2520%2520field%3A%2520doc_count%250A%2520%2520%2520%2520%2520%2520type%3A%2520quantitative%250A%2520%2520%2520%2520%2520%2520axis%3A%2520%257Btitle%3A%2520%2522Document%2520count%2522%257D%250A%2520%2520%2520%2520%257D%250A%2520%2520%257D%250A%257D%250A%21%27%29%2Ctitle%3A%21%27%21%27%2Ctype%3Avega%2CuiState%3A%28%29%29%29%2CgridData%3A%28h%3A15%2Ci%3Aa08de471-c9b0-426f-b54b-5a63f00848c2%2Cw%3A24%2Cx%3A0%2Cy%3A0%29%2CpanelIndex%3Aa08de471-c9b0-426f-b54b-5a63f00848c2%2Ctype%3Avisualization%2Cversion%3A%21%277.13.0%21%27%29%2C%28embeddableConfig%3A%28attributes%3A%28references%3A%21%21%28%28id%3A%21%2790943e30-9a47-11e8-b64d-95841ca0b247%21%27%2Cname%3Aindexpattern-datasource-current-indexpattern%2Ctype%3Aindex-pattern%29%2C%28id%3A%21%2790943e30-9a47-11e8-b64d-95841ca0b247%21%27%2Cname%3Aindexpattern-datasource-layer-efee8f62-ff35-4947-ba88-650a385b2da1%2Ctype%3Aindex-pattern%29%29%2Cstate%3A%28datasourceStates%3A%28indexpattern%3A%28layers%3A%28efee8f62-ff35-4947-ba88-650a385b2da1%3A%28columnOrder%3A%21%21%28%21%27429a63dc-8996-4b87-9a9e-67aab91b4b52%21%27%2C%21%2784dc44ce-5cab-4bc6-a5ed-21543ded2c05%21%27%29%2Ccolumns%3A%28%21%27429a63dc-8996-4b87-9a9e-67aab91b4b52%21%27%3A%28dataType%3Adate%2CisBucketed%3A%21%21t%2Clabel%3Atimestamp%2CoperationType%3Adate_histogram%2Cparams%3A%28interval%3Aauto%29%2Cscale%3Ainterval%2CsourceField%3Atimestamp%29%2C%21%2784dc44ce-5cab-4bc6-a5ed-21543ded2c05%21%27%3A%28dataType%3Anumber%2CisBucketed%3A%21%21f%2Clabel%3A%21%27Count%2520of%2520records%21%27%2CoperationType%3Acount%2Cscale%3Aratio%2CsourceField%3ARecords%29%29%2CincompleteColumns%3A%28%29%29%29%29%29%2Cfilters%3A%21%21%28%29%2Cquery%3A%28language%3Akuery%2Cquery%3A%21%27%21%27%29%2Cvisualization%3A%28axisTitlesVisibilitySettings%3A%28x%3A%21%21t%2CyLeft%3A%21%21t%2CyRight%3A%21%21t%29%2CfittingFunction%3ANone%2CgridlinesVisibilitySettings%3A%28x%3A%21%21t%2CyLeft%3A%21%21t%2CyRight%3A%21%21t%29%2Clayers%3A%21%21%28%28accessors%3A%21%21%28%21%2784dc44ce-5cab-4bc6-a5ed-21543ded2c05%21%27%29%2ClayerId%3Aefee8f62-ff35-4947-ba88-650a385b2da1%2Cposition%3Atop%2CseriesType%3Abar_stacked%2CshowGridlines%3A%21%21f%2CxAccessor%3A%21%27429a63dc-8996-4b87-9a9e-67aab91b4b52%21%27%29%29%2Clegend%3A%28isVisible%3A%21%21t%2Cposition%3Aright%29%2CpreferredSeriesType%3Abar_stacked%2CtickLabelsVisibilitySettings%3A%28x%3A%21%21t%2CyLeft%3A%21%21t%2CyRight%3A%21%21t%29%2CvalueLabels%3Ahide%29%29%2Ctitle%3A%21%27%21%27%2Ctype%3Alens%2CvisualizationType%3AlnsXY%29%2Cenhancements%3A%28%29%29%2CgridData%3A%28h%3A15%2Ci%3A%21%2737e419ab-90eb-4b8a-a116-6edfb54112cc%21%27%2Cw%3A24%2Cx%3A24%2Cy%3A0%29%2CpanelIndex%3A%21%2737e419ab-90eb-4b8a-a116-6edfb54112cc%21%27%2Ctype%3Alens%2Cversion%3A%21%277.13.0%21%27%29%2C%28embeddableConfig%3A%28enhancements%3A%28%29%29%2CgridData%3A%28h%3A15%2Ci%3Aebef68aa-628c-41d0-97aa-ebb79aaf9ee9%2Cw%3A24%2Cx%3A0%2Cy%3A15%29%2Cid%3Aa1f43070-21cd-11ec-ba28-234eedb64ef3%2CpanelIndex%3Aebef68aa-628c-41d0-97aa-ebb79aaf9ee9%2Ctype%3Alens%2Cversion%3A%21%277.13.0%21%27%29%2C%28embeddableConfig%3A%28attributes%3A%28references%3A%21%21%28%28id%3A%21%2790943e30-9a47-11e8-b64d-95841ca0b247%21%27%2Cname%3Aindexpattern-datasource-current-indexpattern%2Ctype%3Aindex-pattern%29%2C%28id%3A%21%2790943e30-9a47-11e8-b64d-95841ca0b247%21%27%2Cname%3Aindexpattern-datasource-layer-502e3b69-3a8e-472f-adb6-06ada2dacf42%2Ctype%3Aindex-pattern%29%29%2Cstate%3A%28datasourceStates%3A%28indexpattern%3A%28layers%3A%28%21%27502e3b69-3a8e-472f-adb6-06ada2dacf42%21%27%3A%28columnOrder%3A%21%21%28%21%275449943f-9508-44a9-9b26-eeb5a4eb7057%21%27%2Ce43fca8a-38dc-45c5-a710-09f5e8ff2804%2Cff9a0a75-7a62-48e2-a603-b0ef9e1f2212%29%2Ccolumns%3A%28%21%275449943f-9508-44a9-9b26-eeb5a4eb7057%21%27%3A%28dataType%3Adate%2CisBucketed%3A%21%21t%2Clabel%3Atimestamp%2CoperationType%3Adate_histogram%2Cparams%3A%28interval%3Aauto%29%2Cscale%3Ainterval%2CsourceField%3Atimestamp%29%2Ce43fca8a-38dc-45c5-a710-09f5e8ff2804%3A%28dataType%3Anumber%2CisBucketed%3A%21%21f%2Clabel%3A%21%27Average%2520of%2520machine.ram%21%27%2CoperationType%3Aavg%2Cparams%3A%28format%3A%28id%3Anumber%2Cparams%3A%28decimals%3A2%29%29%29%2Cscale%3Aratio%2CsourceField%3Amachine.ram%29%2Cff9a0a75-7a62-48e2-a603-b0ef9e1f2212%3A%28dataType%3Anumber%2CisBucketed%3A%21%21f%2Clabel%3A%21%27Moving%2520avg%2520of%2520Average%2520of%2520machine.ram%21%27%2CoperationType%3Amoving_avg%2Cparams%3A%28format%3A%28id%3Anumber%2Cparams%3A%28decimals%3A2%29%29%2Cwindow%3A5%29%2Creferences%3A%21%21%28e43fca8a-38dc-45c5-a710-09f5e8ff2804%29%2Cscale%3Aratio%29%29%2CincompleteColumns%3A%28%29%29%29%29%29%2Cfilters%3A%21%21%28%29%2Cquery%3A%28language%3Akuery%2Cquery%3A%21%27%21%27%29%2Cvisualization%3A%28axisTitlesVisibilitySettings%3A%28x%3A%21%21t%2CyLeft%3A%21%21t%2CyRight%3A%21%21t%29%2CfittingFunction%3ANone%2CgridlinesVisibilitySettings%3A%28x%3A%21%21t%2CyLeft%3A%21%21t%2CyRight%3A%21%21t%29%2Clayers%3A%21%21%28%28accessors%3A%21%21%28ff9a0a75-7a62-48e2-a603-b0ef9e1f2212%29%2ClayerId%3A%21%27502e3b69-3a8e-472f-adb6-06ada2dacf42%21%27%2Cposition%3Atop%2CseriesType%3Abar_stacked%2CshowGridlines%3A%21%21f%2CxAccessor%3A%21%275449943f-9508-44a9-9b26-eeb5a4eb7057%21%27%2CyConfig%3A%21%21%28%28axisMode%3Aleft%2CforAccessor%3Aff9a0a75-7a62-48e2-a603-b0ef9e1f2212%29%29%29%29%2Clegend%3A%28isVisible%3A%21%21t%2Cposition%3Aright%29%2CpreferredSeriesType%3Abar_stacked%2CtickLabelsVisibilitySettings%3A%28x%3A%21%21t%2CyLeft%3A%21%21t%2CyRight%3A%21%21t%29%2CvalueLabels%3Ahide%29%29%2Ctitle%3A%21%27new%2520vis%21%27%2CvisualizationType%3AlnsXY%29%29%2CgridData%3A%28h%3A15%2Ci%3A%21%276c9f4032-59e1-40e2-8ef7-f0ddf5e83d14%21%27%2Cw%3A24%2Cx%3A24%2Cy%3A15%29%2CpanelIndex%3A%21%276c9f4032-59e1-40e2-8ef7-f0ddf5e83d14%21%27%2Ctype%3Alens%2Cversion%3A%21%277.13.0%21%27%29%29%2Cquery%3A%28language%3Akuery%2Cquery%3A%21%27%21%27%29%2Ctags%3A%21%21%28%29%2CtimeRestore%3A%21%21f%2Ctitle%3A%21%27Test%25203%21%27%2CviewMode%3Aedit%29%27%2Ctitle%3A%27Test%203%27%29

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
There could be scenarios when we should crash Low Med It seems unlikely that there are such scenarios in the browser environment

@jloleysens jloleysens added (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Reporting Services v7.16.0 v7.15.1 labels Sep 30, 2021
@jloleysens jloleysens requested review from a team as code owners September 30, 2021 14:52
@elasticmachine
Copy link
Contributor

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

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@tsullivan
Copy link
Member

@elasticmachine merge upstream

map((err) => {
logger.error(
i18n.translate('xpack.reporting.browsers.chromium.pageErrorDetected', {
defaultMessage: `Reporting encountered an error on the page: {err}`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this log line be warning since the error is ignored? And/or should the content mention that we are ignoring the error?

Catching and logging the error makes this similar to the pageRequestFailed$ observable.

I am in favor of updating the content on line 228 to remove the This error will be ignored text. That has confused users in times when the report eventually does fail. But I do think this log level should be warning since we're not throwing the error after logging it.

Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a comment about making the tone of the message similar to line 228

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@jloleysens
Copy link
Contributor Author

Thanks for the review @tsullivan , I think I've addressed your feedback. Would mind taking another look?

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@jloleysens jloleysens merged commit 75983cf into elastic:master Oct 11, 2021
@jloleysens jloleysens deleted the reporting/dont-crash-on-unhandled-exceptions branch October 11, 2021 10:59
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 11, 2021
* move uncaught exception out of exit$

* reintroduce original error, but as a log instead

* change log level: error -> warning. also update copy to make it explicit that the error will be ignored

Co-authored-by: Kibana Machine <[email protected]>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 11, 2021
* move uncaught exception out of exit$

* reintroduce original error, but as a log instead

* change log level: error -> warning. also update copy to make it explicit that the error will be ignored

Co-authored-by: Kibana Machine <[email protected]>
jloleysens added a commit that referenced this pull request Oct 11, 2021
* move uncaught exception out of exit$

* reintroduce original error, but as a log instead

* change log level: error -> warning. also update copy to make it explicit that the error will be ignored

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
jloleysens added a commit that referenced this pull request Oct 11, 2021
* move uncaught exception out of exit$

* reintroduce original error, but as a log instead

* change log level: error -> warning. also update copy to make it explicit that the error will be ignored

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes v7.15.1 v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants