-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Improve fatal error message #186609
Improve fatal error message #186609
Conversation
/ci |
@elasticmachine merge upstream |
var errorTitle = document.querySelector('[data-error-message-title]').dataset.errorMessageTitle; | ||
var errorText = document.querySelector('[data-error-message-text]').dataset.errorMessageText; | ||
var errorReload = document.querySelector('[data-error-message-reload]').dataset.errorMessageReload; | ||
|
||
var err = document.createElement('div'); | ||
err.style.textAlign = 'center'; | ||
err.style.padding = '120px 20px'; | ||
err.style.fontFamily = 'Inter, BlinkMacSystemFont, Helvetica, Arial, sans-serif'; | ||
|
||
var errorTitleEl = document.createElement('h1'); | ||
errorTitleEl.innerText = errorTitle; | ||
errorTitleEl.style.margin = '20px'; | ||
|
||
var errorTextEl = document.createElement('p'); | ||
errorTextEl.innerText = errorText; | ||
errorTextEl.style.margin = '20px'; | ||
|
||
var errorReloadEl = document.createElement('button'); | ||
errorReloadEl.innerText = errorReload; | ||
errorReloadEl.onclick = function () { | ||
location.reload(); | ||
}; | ||
errorReloadEl.setAttribute('style', | ||
'cursor: pointer; padding-inline: 12px; block-size: 40px; font-size: 1rem; line-height: 1.4286rem; border-radius: 6px; min-inline-size: 112px; color: rgb(255, 255, 255); background-color: rgb(0, 119, 204); outline-color: rgb(0, 0, 0); border:none' | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're in 2008. jQuery just got out of beta, and developers are still forced to have their "code" work on IE5.5 and IE6. (jocking, doc.querySelector wasn't a thing for IE6)
defaultMessage: | ||
'Elastic did not load properly. Check the server output for more information.', | ||
'Please reload this page. If the issue persists, check the server logs.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should we mention the browser's console as well?
AFAIK, more often than not, this error is likely an issue loading assets, and it will probably be more obvious to start with the browser's console logs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
History
To update your PR or re-run it, just comment with: |
## Summary This PR aims to improve the message shown to users when Kibana can't be started due to disabled DOM storage (#121189). The visuals here follow the same pattern as other fatal errors (see #186609) ![image](https://github.com/user-attachments/assets/19832830-49e3-4789-9b83-0c1f14d7980d) The `isDomStorageDisabled` check has to be done before `CoreService` gets instantiated because of issues described below. Closes: #121189 ## The issue What actually happens when you disable all cookies in a browser? Aside from cookies, the browser disables the whole DOM storage - `localStorage` and `sessionStorage`. Trying to access those will result in an error. `getSessionId`https://github.com/elastic/kibana/blob/3bc5e2db73799dc9c7831b6f9da4a52063cf112f/packages/core/analytics/core-analytics-browser-internal/src/get_session_id.ts#L17 and `isSidenavCollapsed$`https://github.com/elastic/kibana/blob/3bc5e2db73799dc9c7831b6f9da4a52063cf112f/packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx#L91 Both of those try to access either `localStorage` or `sessionStorage` and both of those are triggered when you create an instance of `CoreSystem` which gets instantiated in `kbn_bootstrap` https://github.com/elastic/kibana/blob/6ef03697460aba0d3774c0c03fb7fb58c76c00bd/packages/core/root/core-root-browser-internal/src/kbn_bootstrap.ts#L42 Trying to access DOM storage in `CoreSystem` will cause it to throw an error and this means that `FatalErrorService`https://github.com/elastic/kibana/blob/6ef03697460aba0d3774c0c03fb7fb58c76c00bd/packages/core/fatal-errors/core-fatal-errors-browser-internal/src/fatal_errors_service.tsx#L32 will never instantiate and the `failure`https://github.com/elastic/kibana/blob/6ef03697460aba0d3774c0c03fb7fb58c76c00bd/packages/core/rendering/core-rendering-server-internal/src/bootstrap/render_template.ts#L68 function which styles the errors and makes them visible will never trigger and all the user will see is permament `Loading Kibana` spinner. Wrapping `getSessionId` and `isSidenavCollapsed$` in `try-catch` block allows `FatalErrorService` to work properly, which will catch an unhandled exception (`Detected an unhandled Promise rejection.`) with an error about `sessionStorage` being disabled, which gets thrown by `LicensingPlugin` (and possibly in other places). This is not an actual solution though - this behavior would happen again if another line of code trying to access DOM storage gets added to `CoreSystem`. I think it would be best to handle this directly in `kbn_bootstrap.ts` by some check like the one below: ```javascript const isDOMStorageDisabled = () => { try { const key = 'kbn_bootrasrap_domStorageEnabled'; sessionStorage.setItem(key, 'true'); sessionStorage.removeItem(key); return false; } catch (e) { return true; } }; const domStorageDisabled = isDOMStorageDisabled() /* ...some additonal logic */ ``` This would then require some error displaying logic that doesn't use `FatalErrorService`. Looking for some feedback on how to properly solve this.
## Summary This PR aims to improve the message shown to users when Kibana can't be started due to disabled DOM storage (elastic#121189). The visuals here follow the same pattern as other fatal errors (see elastic#186609) ![image](https://github.com/user-attachments/assets/19832830-49e3-4789-9b83-0c1f14d7980d) The `isDomStorageDisabled` check has to be done before `CoreService` gets instantiated because of issues described below. Closes: elastic#121189 ## The issue What actually happens when you disable all cookies in a browser? Aside from cookies, the browser disables the whole DOM storage - `localStorage` and `sessionStorage`. Trying to access those will result in an error. `getSessionId`https://github.com/elastic/kibana/blob/3bc5e2db73799dc9c7831b6f9da4a52063cf112f/packages/core/analytics/core-analytics-browser-internal/src/get_session_id.ts#L17 and `isSidenavCollapsed$`https://github.com/elastic/kibana/blob/3bc5e2db73799dc9c7831b6f9da4a52063cf112f/packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx#L91 Both of those try to access either `localStorage` or `sessionStorage` and both of those are triggered when you create an instance of `CoreSystem` which gets instantiated in `kbn_bootstrap` https://github.com/elastic/kibana/blob/6ef03697460aba0d3774c0c03fb7fb58c76c00bd/packages/core/root/core-root-browser-internal/src/kbn_bootstrap.ts#L42 Trying to access DOM storage in `CoreSystem` will cause it to throw an error and this means that `FatalErrorService`https://github.com/elastic/kibana/blob/6ef03697460aba0d3774c0c03fb7fb58c76c00bd/packages/core/fatal-errors/core-fatal-errors-browser-internal/src/fatal_errors_service.tsx#L32 will never instantiate and the `failure`https://github.com/elastic/kibana/blob/6ef03697460aba0d3774c0c03fb7fb58c76c00bd/packages/core/rendering/core-rendering-server-internal/src/bootstrap/render_template.ts#L68 function which styles the errors and makes them visible will never trigger and all the user will see is permament `Loading Kibana` spinner. Wrapping `getSessionId` and `isSidenavCollapsed$` in `try-catch` block allows `FatalErrorService` to work properly, which will catch an unhandled exception (`Detected an unhandled Promise rejection.`) with an error about `sessionStorage` being disabled, which gets thrown by `LicensingPlugin` (and possibly in other places). This is not an actual solution though - this behavior would happen again if another line of code trying to access DOM storage gets added to `CoreSystem`. I think it would be best to handle this directly in `kbn_bootstrap.ts` by some check like the one below: ```javascript const isDOMStorageDisabled = () => { try { const key = 'kbn_bootrasrap_domStorageEnabled'; sessionStorage.setItem(key, 'true'); sessionStorage.removeItem(key); return false; } catch (e) { return true; } }; const domStorageDisabled = isDOMStorageDisabled() /* ...some additonal logic */ ``` This would then require some error displaying logic that doesn't use `FatalErrorService`. Looking for some feedback on how to properly solve this. (cherry picked from commit 3a8bd70)
Summary
Close https://github.com/elastic/kibana-team/issues/948
Makes the error message when Kibana fails to load less scary