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

Gracefully handle 404s when attempting to load assets #162338

Closed
jloleysens opened this issue Jul 20, 2023 · 13 comments
Closed

Gracefully handle 404s when attempting to load assets #162338

jloleysens opened this issue Jul 20, 2023 · 13 comments
Labels
Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@jloleysens
Copy link
Contributor

jloleysens commented Jul 20, 2023

Kibana loads many assets just-in-time to ensure we only load on the UI what is necessary. These assets are hosted in */${buildNum}/bundles/* (where buildNum is a number like 6812).

When switching out for a different Kibana version "on the fly" and not enforcing client version checks (see here) then we should consider handling in some way.

Note: this is not the same as a check for a new version of Kibana being available. This is to gracefully handle the error state that a user can get into if they try to open a section of Kibana that is just not available.

CC @rayafratkina

@jloleysens jloleysens added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Jul 20, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@pgayvallet
Copy link
Contributor

pgayvallet commented Jul 24, 2023

when attempting to load assets

Just to clarify, by assets, are we thinking globally of Kibana assets (e.g fonts, images, and so on - which aren't necessarily under this */${buildNum}/ prefix), or specifically of webpack chunks?

Because the scope of the issue seems fairly different to me depending on this clarification.

FWIW, I just took a quick look, and it seems there isn't any "hook" on the webpack chunk load API.

The official webpack documentation recommends to just monkey patch the method for such needs: https://webpack.js.org/api/module-variables/#__webpack_chunk_load__-webpack-specific. But I think it we can work with that.

This is to gracefully handle the error state that a user can get into if they try to open a section of Kibana that is just not available

What would be the expected "graceful handle" here? I assume we'd like something very similar to what we want when detecting a server/ui version mismatch, is that correct? So a page reload, or an error modal, or something?

@jloleysens
Copy link
Contributor Author

Good questions!

*/${buildNum}/ prefix

Yeah, specifically things affected by this is what I had in mind. So not all assets, as you point out.

FWIW, I just took a quick look, and it seems there isn't any "hook" ... monkey patch the method for such needs

Nice! Yeah I was hoping for something like this 👏🏻

@afharo
Copy link
Member

afharo commented Aug 7, 2023

I wonder if it's more worth spending our efforts on supporting CDNs.

My main worry is: we let the user know they should refresh, but they first want to save their visualization. They click "save", and it will try to open the Save Modal... but that model requires a new chunk... but the chunk is not loaded yet and returns 404... what do we do?

@jloleysens
Copy link
Contributor Author

I wonder if it's more worth spending our efforts on supporting CDNs.

Yeah, you're right this would be "solved" by using a CDN (or actually separating static assets from Kibana). The idea here was to find the lowest effort solution to at least be able to tell users they need to refresh.

Unfortunately downtime (in the form of restart) will occur atm. The idea is just to give users a hint rather than hard crashing the UI.

@jloleysens
Copy link
Contributor Author

Just wanted to give an update: looks like using something like suggested with webpack could be a feasible solution. But might take some further investigation given how we bundle each plugin and so we'd need to override the chunk loader function for all plugins.

We might return to this down the line, but it seems likely that we'd rather solve the root of the 404 problem in the way that @afharo described (giving the best UX).

@Dosant
Copy link
Contributor

Dosant commented Aug 10, 2023

My main worry is: we let the user know they should refresh, but they first want to save their visualization. They click "save", and it will try to open the Save Modal... but that model requires a new chunk... but the chunk is not loaded yet and returns 404... what do we do?

I agree with @afharo, I think there is no way to handle this on UI gracefully. If chunks become unavailable, I'd consider the client completely broken until the user reloads. We can show an error message, but unsaved data and any client state may be lost. This also would be very intrusive to ask to reload.

Here is a couple issues @bhavyarm bumped into while testing, I assume this is caused by bundles becoming unavailable #163341, #163334. I myself hit it multiple times on qa (not sure how often we redeloy there), here is another slack thread with the issue

If not a CDN, can we bundle previous, for example, 10 builds together with a new version so the chunks stay available after the deploy of the newer version?

@vadimkibana
Copy link
Contributor

vadimkibana commented Aug 10, 2023

A week ago in the Serverless sync I was suggesting to upload every version of Kibana (every commit in main) build artifacts into its own folder on a CDN, then all static assets are served from that CDN. It would also solve this problem, in addition to other benefits.

Solving this in UI could help, but essentially it would be a hard reload. When we detect a version mismatch we need to immediately reload the webapp or show a message to user to do it manually while blocking all other UI actions (which is essentially the same as automatically doing the hard reload).

I wonder if it's more worth spending our efforts on supporting CDNs.

Yes, that is something years overdue in Kibana. It is a very low handing fruit with big benefits.


Doing any Webpack hack is just increasing the tech debt and is no less effort than setting up a CDN.

@lukeelmers
Copy link
Member

Related #163671

@pgayvallet
Copy link
Contributor

Doing any Webpack hack is just increasing the tech debt and is no less effort than setting up a CDN.

Am I the only one to disagree on the said "same-amount" of effort for these two options?

Hacking something around the webpack loader should be a matter of days.

Setting up a CDN, having the mechanism to upload the assets to it during ci build/distribution, changing Kibana to consume assets from it, is a matter of what? weeks? months? If it was a matter of days, we would have done it already. Can I just remind everyone that today we don't even know which tool/cdn we would be using to serve the assets?

Knowing that, how can we say the webpack workaround is no less effort than setting up a CDN?

Don't get me wrong, everyone will agree that the CDN is the correct long term approach.

But, again, today in our MKI environments, a missing chunk due to ui/server version mismatch will causes an error fully and silently breaking the UI. So, well, spending some effort on a temporary solution, even if it's just about force-reloading the UI when a 404 chunk is attempted to be loaded, seems like a fairly quick win in term of user experience to me.

I'd love to be proven wrong, but ihmo we won't have a CDN for the MVP (and I doubt we will have it before EOY).

But if I'm the only one with this opinion, I'll gladly follow the majority.

@afharo
Copy link
Member

afharo commented Sep 11, 2023

Just want to highlight @lukeelmers's comment #162338 (comment)

It links to an issue where we would improve the current error page to be more UX. Do you think that would solve our current concerns while we get to the CDN approach?

@lukeelmers
Copy link
Member

Do you think that would solve our current concerns while we get to the CDN approach?

Yes, IMO if we can simply improve the UX for when chunks currently fail (and give users a clear prompt, like to refresh the page), that would be sufficient until we can get to a CDN.

@rayafratkina
Copy link
Contributor

Not planning to do this in the near term

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests

8 participants