iron-meteor / iron-router

A client and server side router designed specifically for Meteor.
MIT License
1.98k stars 413 forks source link

Please prettify the error page #1481

Closed cdcv closed 8 years ago

cdcv commented 8 years ago

Please create an error page that is more suited to production environments. We realize that we don't want to be having Iron:Router errors in production environments, but it has happened and... well... it ain't pretty. That's sure not what we want our users to see if something goes wrong. Our experience has been that the error page (in the several thousand times that we've seen it) has never presented any useful information. Please either change it to look less error-full or, and this would be great, allow us to provide content or a URL link for what will be seen if this page tries to display. Just to be clear, I'm talking about the page that comes up if (e.g.) the code includes duplicate routes, the JS code hasn't loaded correctly, etc.

Thanks

chrisbutler commented 8 years ago

@cdcv can you provide an example of what page you're talking about?

cdcv commented 8 years ago

Thanks very much for your response, and for your help with this. Here is an example of the screen that comes up while our Meteor app loads the first time after being downloaded from the app store. BTW, guidance on how to avoid this happening would also be fantastic. After some time (which seems like it may depend on the total volume of the code) the app gets started. This, however, would obviously put off any user. If we could customize this to be a nice pretty ‘loading’ screen, that would be infinitely better.

CdC

On Nov 27, 2015, at 8:25 PM, Chris Butler notifications@github.com wrote:

@cdcv https://github.com/cdcv can you provide an example of what page you're talking about?

— Reply to this email directly or view it on GitHub https://github.com/iron-meteor/iron-router/issues/1481#issuecomment-160247328.

chrisbutler commented 8 years ago

@cdcv hey, i'm not seeing anything here

cdcv commented 8 years ago

Hmmm. I sent a reply email with an attached image of the screen. Is there a way to upload here? I am currently having the issue on a mobile device, so no easy way to copy/paste. Could also email it to you directly. Thanks.

chrisbutler commented 8 years ago

you could upload a screenshot to something like imgur. but from what you're describing it doesn't really sound like this is an issue with iron router per se

cdcv commented 8 years ago

Here is the screen that I'm talking about. Thanks

http://www.cdc-ventures.com/images/IMG_5983.JPG

chrisbutler commented 8 years ago

@cdcv interesting. that's the screen you get when no routes are defined... are you doing something like loading all of the routes dynamically?

chrisbutler commented 8 years ago

hmm, seems like the same issue as https://github.com/iron-meteor/iron-router/issues/1290?

cdcv commented 8 years ago

Not sure if it's the same as 1290. It seems that Meteor is still in the midst of loading the routes, and when it finishes the screen goes away and the app commences (or sometimes it crashes). What I was actually suggesting though was just a nicer looking error screen, or the ability to control it's content, so that when this does happen it's better for users...

cdcv commented 8 years ago

Looking more closely, this isn't the same as 1290, as removing code part by part doesn't solve it. It seems to be caused by the sheer volume of code. For example, removing either half of a bunch of meteor code will make the problem go away, but if both halves are present then this is the result. Again, it seems to be due to the time required to load all the code before the routes become valid and the app can start.

cdcv commented 8 years ago

Sometimes get the same error screen if other things prevent the routes from correctly loading, eg certain js issues.

chrisbutler commented 8 years ago

@cdcv having a custom error screen could be a useful feature, but it seems like there's an actual issue with your app structure that causes you to see this screen... or some deeper underlying issue with IR and cordova?

cdcv commented 8 years ago

A custom error screen would be a big help. And I'd sure appreciate it if you make one (guessing that others would too). I don't think that the issue we are having is a traditional problem with the app code (other than that there is too much of it), but perhaps is better thought of as 'deeper'. You might try creating a large app with a very large number of templates/js files, and I suspect that will produce the effect, though I haven't tried it. I know that I've gotten this same error screen in other ways as well, but can't remember what they were at the moment (just remember that the actual error didn't have anything to do with what it says on the screen. Thanks for your help.

chrisbutler commented 8 years ago

@cdcv are you using reload-on-resume, by chance? or serving assets out of the public folder? there are also a few other possibilities that might be causing this

generally i think that screen actually looks pretty good for the purpose it serves, and also it really doesn't make sense to add a "feature" which is really a work-around for one specific edge case that can't be shown to be an actual bug and not user error.

does that make sense?

cdcv commented 8 years ago

Hi Chris. We're not using reload-on-resume. We have about 17MB in our public folder. Is that too much? Is there some better way to serve it? We've seen this same error in multiple different applications with different code bases, so I don't really see it as a totally uncommon edge case: it may become more common as people create larger and more complex Meteor apps. I think that allowing people who use Iron:Router to customize an error screen that their users are seeing would be a pretty good idea. Frankly, the risk of users seeing an unprofessional error screen if things go wrong -- they inevitably do at some point -- would be a sufficient reason for people to have to switch to one of the other routers, even if they didn't want to otherwise. I think that the solution -- adding a custom error screen -- would also be an incredibly simple one to make. I'd like to ask you to make it...

Thanks for your help.

chrisbutler commented 8 years ago

hey @cdcv, that's most likely your problem. in production mode, meteor bundles the entire app, including the public folder... and so your app is waiting for the entire bundle to be loaded before running the code that initializes the routes

the preferred method of serving static assets is to use a 'cdn' (i.e. a separate server or s3)

the problem here is the same as https://github.com/iron-meteor/iron-router/issues/1289#issuecomment-160942314, where the problem occurs completely outside of the router, and detecting this is totally outside of the scope of the package

chrisbutler commented 8 years ago

also, there is already a way to set a custom error screen for actual router errors, which is notFoundTemplate

cdcv commented 8 years ago

I'd like to point out that we are serving it from the public folder precisely because when we were serving the assets from S3, the load times were often too slow due to network traffic etc and we would see the same error. Once again, I'd like to suggest that focusing on the idea that this screen should never be seen is denying the present reality that it is. Thanks on notFoundTemplate. Will look into that. Will that template come up in the circumstance that we're talking about? If so, that may be our solution! Thanks again for your help.

chrisbutler commented 8 years ago

@cdcv you're not getting it. if iron router doesn't find any routes, it's not going to find your code that defines a custom template for the 'noRoutes' condition... (if that feature were to be added)

cdcv commented 8 years ago

OK, then this brings me back to the initial suggestion: please make it possible to customize the iron:router error screen so that it is end-user-appropriate. Perhaps it can have a 'technical' link to the current screen. Thanks again.

chrisbutler commented 8 years ago

that's exactly what i'm saying... customize it how/where? do you not see the problem with your suggestion and why it doesn't work/make sense?

whatever code would need to run to load that customization wouldn't be run because of other problems with your application architecture... same as the routes

cdcv commented 8 years ago

Well I'm afraid I don't. There is currently content/code that produces the existing error screen. You've probably got better ideas for how to solve this than I do, but here are a couple that I can think of: 1) Make this content more end-user-appropriate (rather than coder-appropriate) in the base package -- seems like the best idea. 2) Make this content point/auto-refresh to a pre-defined URL that could be defined earlier in the build cycle, before all of the Meteor JS and assets load, so that it would be available when needed. Perhaps definable in build settings? Not sure when those load. 3) Make it straightforward to edit this content in a local package version (perhaps it already is), perhaps a local package that just adds this mod and references the base package. This seems pretty complex, but might be better than either switching router or forking the whole thing...

cdcv commented 8 years ago

Also, if this error page comes up if there are too many assets, if the page is going to be coder-directed, it would be good to make clear on the error page what the cause might be.

chrisbutler commented 8 years ago

you could certainly install the iron-router package locally and modify the __IronRouterNoRoutes__ template.

that seems to be your best short-term option if you have no intention of actually addressing the issue of loading megabytes of data before your app can actually start

unfortunately, i think there's too much complexity in your suggestions to make it worth trying to add that feature, especially when (whether you want to agree with this or not) this is a screen that will only be shown to end users if there are other problems in your app (which, historically, have been fixable by everyone else who has so far encountered this problem)

if you're able to submit a PR that provides a fix (along with a workable reproduction and documentation) we'd certainly be willing to incorporate that into the package

good luck!

chrisbutler commented 8 years ago

@cdcv there's really no simple way for the router to tell what issue with your code is the show-stopper... and it's really outside the scope of a routing package to try to do this kind of debugging

also, the issue with large public folders is a Meteor one, and the limitations/solutions are known https://github.com/meteor/meteor/issues/4119 https://forums.meteor.com/t/first-visit-loads-are-ridiculously-slow/3028

cdcv commented 8 years ago

Chris, thanks for your help with this. We'll find a solution among all of this -- probably one for short-term and one for long-term. If we end up creating a local version of the router, will send you a PR.

Slanman3755 commented 8 years ago

Still don't agree with the error page.

chrisbutler commented 8 years ago

@Slanman3755 don't agree with it how?

Slanman3755 commented 8 years ago

In a production environment I think there should be a short error message with no styling.

chrisbutler commented 7 years ago

@cdcv @Slanman3755 this exists now