Implement/better warning for url length #6773

spalger · 2016-04-04T22:18:21Z

This change warns the user when they are about to overflow the maximum url length supported by their browser, and then renders a url-overflow specific error page once they do.

Things to note:

Tried and failed to find a single place where this could be detected as it's happening, instead defined the max length ahead of time.
Tried and failed to find a way to "feature detect" the url length limit, had to resort to user-agent matching.
The limits are not configurable
Limits:
- IE/Edge, 2,000 characters
- All other browsers 25,000

w33ble · 2016-04-05T00:10:57Z

src/ui/public/config/defaults.js

+      `,
+    },
+    'url:limit': {
+      value: 2082,


Weren't we seeing IE fail before this point? I thought 2048 was the magic number there...

We were testing the length of the hash, I think... I trust the support doc though.

Oh, right, that's the whole URL length... alrighty then.

w33ble · 2016-04-06T00:04:57Z

I get the error when I hit the hard upper limit, but I only see it in the console, not at the top of Kibana:

Seems like I should see a red error bar at the top too - perhaps it's not being caught properly?

spalger · 2016-04-06T00:07:36Z

Yeah, perhaps we should just fatal 😕

w33ble · 2016-04-06T00:08:34Z

I think just showing the error message on the screen would be good enough really. As long as the user knows why something isn't happening, that's really the goal here.

throw new TypeError(`
  The URL has gotten too big and kibana can no longer
  continue. Please refresh to return to your previous state.
`);

Maybe a notify.error instead, or in addition?

…terWarningForUrlLength

spalger · 2016-04-06T14:54:22Z

I took a stab at a custom error handler/route that displays the URL and gives tips for fixing the problem. Thoughts?

w33ble · 2016-04-06T20:04:03Z

@spalger I like that, and I like that it links to the parts of Kibana that you can use to get the issue fixed.

This is looking good, but I'd get some additional input on the wording on the error page. I like it though.

spalger · 2016-04-07T00:17:15Z

src/ui/public/error_url_overflow/error_url_overflow.html

+  <h3>Tips</h3>
+  <ul>
+    <li>
+      Saved objects can sometimes persist some unexpectedly large values that get put into the url when they load. You may be able to edit these saved objects in the <a href="#/settings/objects">"objects" section of the settings app</a>.


@palecur could you help us with wording here?

rashidkpc · 2016-04-14T23:52:54Z

I'm concerned that this will trigger often in browsers that do not have the URL size limit. I'm usually completely against browser specific behaviors, but in this case I would like us to handle IE specifically and ask the user to reduce the size of the dashboard using another browser, informing them that IE in particular is problematic.

Otherwise we're going to throw this warning at a huge number of users who don't need to take any action, and make them go into the big scary advanced settings list to fix it.

spalger · 2016-04-26T17:07:33Z

Reworded the description, removed the unused config default, and responded to questions inline

rashidkpc · 2016-04-27T15:36:20Z

@spalger couple small changes and this is GTG

…terWarningForUrlLength

--------- **Commit 1:** [state] add configurable warning level based on url length * Original sha: 55db90d * Authored by spalger <spalger@users.noreply.github.com> on 2016-04-04T21:57:45Z **Commit 2:** [state] add a hard length limit that will start throwing errors * Original sha: 64699aa * Authored by spalger <spalger@users.noreply.github.com> on 2016-04-04T22:14:06Z **Commit 3:** Merge branch 'master' of github.com:elastic/kibana into implement/betterWarningForUrlLength * Original sha: 4c70d69 * Authored by spalger <spalger@users.noreply.github.com> on 2016-04-06T07:05:47Z **Commit 4:** [dashboard] cleanup quietly to prevent error * Original sha: 1dace5c * Authored by spalger <spalger@users.noreply.github.com> on 2016-04-06T08:50:55Z **Commit 5:** [errorview] add url overflow display * Original sha: 8b4ebf5 * Authored by spalger <spalger@users.noreply.github.com> on 2016-04-06T08:51:47Z **Commit 6:** [errorview] persist the overflow url so that refresh works * Original sha: e308db9 * Authored by spalger <spalger@users.noreply.github.com> on 2016-04-06T09:05:13Z **Commit 7:** Merge branch 'master' of github.com:elastic/kibana into implement/betterWarningForUrlLength * Original sha: 121e4f2 * Authored by spalger <email@spalger.com> on 2016-04-15T18:49:12Z **Commit 8:** [config] remove url limit config, it should adapt automatically * Original sha: 281b38b * Authored by spalger <email@spalger.com> on 2016-04-15T23:38:05Z **Commit 9:** [chrome] rework url overflow detection The previous version of this pr relied on the State service to catch times when the URL would grow out of control. While overflows will commonly occur in the State service, this didn't handle urls that were navigated to using a link. They worked because the state service would eventually be called, but the failure was unexpected and required interaction to trigger. This new approach does the checking at a higher level, in the chrome. We also removed the `url:limit` configuration value in favor of browser detection (I was not able to find or come up with a way to quietly and quickly feature detect this). The new limits are 2000 characters for IE and 25000 for all other browsers. * Original sha: 116521c * Authored by spalger <email@spalger.com> on 2016-04-15T23:40:18Z **Commit 10:** Merge branch 'master' of github.com:elastic/kibana into implement/betterWarningForUrlLength * Original sha: aa030d7 * Authored by spalger <email@spalger.com> on 2016-04-26T16:51:22Z **Commit 11:** [ui/config] remove unused default * Original sha: b333c51 * Authored by spalger <email@spalger.com> on 2016-04-26T16:52:29Z **Commit 12:** Merge branch 'master' of github.com:elastic/kibana into implement/betterWarningForUrlLength * Original sha: 133d7e5 * Authored by spalger <email@spalger.com> on 2016-04-27T23:17:44Z **Commit 13:** [urlOverflow] assign magic numbers to variables * Original sha: 8b97881 * Authored by spalger <email@spalger.com> on 2016-04-27T23:24:17Z

[backport] PR #6773 to 4.x - IE length warning

mrjameshamilton · 2017-12-11T14:08:03Z

Our users have also come across this and changing browsers is not always possible. It would be better to fix this properly by not using the URL to save the state.

w33ble · 2017-12-11T21:28:15Z

@mrjameshamilton In Kibana 5.0.0 and later, you can enable state:storeInSessionStorage to remove application state from the URL completely. Enabling this will allow you to use very complex dashboards with IE without any issues. See #8022 for more details.

spalger added 2 commits April 4, 2016 15:05

[state] add configurable warning level based on url length

55db90d

[state] add a hard length limit that will start throwing errors

64699aa

spalger added review v5.0.0 labels Apr 4, 2016

spalger assigned w33ble Apr 4, 2016

w33ble reviewed Apr 5, 2016
View reviewed changes

w33ble added updates_needed and removed review labels Apr 6, 2016

w33ble assigned spalger and unassigned w33ble Apr 6, 2016

spalger added 4 commits April 6, 2016 00:05

Merge branch 'master' of github.com:elastic/kibana into implement/bet…

4c70d69

…terWarningForUrlLength

[dashboard] cleanup quietly to prevent error

1dace5c

[errorview] add url overflow display

8b4ebf5

[errorview] persist the overflow url so that refresh works

e308db9

spalger mentioned this pull request Apr 6, 2016

Fatal Error in IE Browser when doing Multiple Pinned Filters #6790

Closed

spalger assigned w33ble and unassigned spalger Apr 6, 2016

w33ble added review v4.5.1 v5.0.0-alpha2 and removed updates_needed labels Apr 6, 2016

w33ble assigned spalger and unassigned w33ble Apr 6, 2016

spalger reviewed Apr 7, 2016
View reviewed changes

[ui/config] remove unused default

b333c51

spalger assigned rashidkpc and unassigned spalger Apr 26, 2016

epixa removed the v5.0.0-alpha2 label Apr 27, 2016

rashidkpc assigned spalger and unassigned rashidkpc Apr 27, 2016

spalger added 2 commits April 27, 2016 18:17

Merge branch 'master' of github.com:elastic/kibana into implement/bet…

133d7e5

…terWarningForUrlLength

[urlOverflow] assign magic numbers to variables

8b97881

spalger merged commit d2b758e into elastic:master Apr 27, 2016

This was referenced Apr 27, 2016

[backport] PR #6773 to 4.x #7078

Closed

[backport] PR #6773 to 4.5 #7079

Closed

epixa added the v5.0.0-alpha2 label Apr 29, 2016

epixa added v4.5.2 and removed v4.5.1 labels May 14, 2016

epixa removed the v4.5.2 label Jun 6, 2016

elastic-jasper mentioned this pull request Aug 15, 2016

[backport] PR #6773 to 4.x - IE length warning #7999

Merged

epixa added a commit that referenced this pull request Aug 19, 2016

Merge pull request #7999 from elastic/jasper/backport/6773/4.x

b7a81df

[backport] PR #6773 to 4.x - IE length warning

cjcenizal mentioned this pull request Jul 8, 2018

Reactify route-based errors #20543

Closed

spalger deleted the implement/betterWarningForUrlLength branch October 18, 2019 17:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement/better warning for url length #6773

Implement/better warning for url length #6773

spalger commented Apr 4, 2016 •

edited

Loading

w33ble Apr 5, 2016

spalger Apr 5, 2016

w33ble Apr 5, 2016

w33ble commented Apr 6, 2016

spalger commented Apr 6, 2016

w33ble commented Apr 6, 2016

spalger commented Apr 6, 2016

w33ble commented Apr 6, 2016

spalger Apr 7, 2016

rashidkpc commented Apr 14, 2016 •

edited

Loading

spalger commented Apr 26, 2016

rashidkpc commented Apr 27, 2016

mrjameshamilton commented Dec 11, 2017

w33ble commented Dec 11, 2017

Implement/better warning for url length #6773

Implement/better warning for url length #6773

Conversation

spalger commented Apr 4, 2016 • edited Loading

w33ble Apr 5, 2016

Choose a reason for hiding this comment

spalger Apr 5, 2016

Choose a reason for hiding this comment

w33ble Apr 5, 2016

Choose a reason for hiding this comment

w33ble commented Apr 6, 2016

spalger commented Apr 6, 2016

w33ble commented Apr 6, 2016

spalger commented Apr 6, 2016

w33ble commented Apr 6, 2016

spalger Apr 7, 2016

Choose a reason for hiding this comment

rashidkpc commented Apr 14, 2016 • edited Loading

spalger commented Apr 26, 2016

rashidkpc commented Apr 27, 2016

mrjameshamilton commented Dec 11, 2017

w33ble commented Dec 11, 2017

spalger commented Apr 4, 2016 •

edited

Loading

rashidkpc commented Apr 14, 2016 •

edited

Loading