meteor-dev-error-overlay icon indicating copy to clipboard operation
meteor-dev-error-overlay copied to clipboard

Triggers if rebuild takes too long

Open lassombra opened this issue 9 years ago • 11 comments

If the rebuild takes longer than jquery timeout (which is uncommon) it will trigger a build error alert.

Also triggers if the server is turned off, which may or may not be an error. Ideally it'll detect the difference between server unreachable and server having errors, and each could be configured to trigger notifications.

lassombra avatar Mar 21 '16 19:03 lassombra

Hmm, yeah I guess we should differentiate between the server being unreachable (immediate failure), long rebuilds (really long response time for the request), and others.

stubailo avatar Mar 21 '16 19:03 stubailo

We already distinguish between server unreachable and server having error here, but maybe the package should behave differently in the former case?

Also if the rebuild is too long, we should be able to see a corresponding error in the callback (something lik 408 Request Time-out). Do you recommend that we don’t display the overlay in this case? (also, do you have a reproducible way to trigger long rebuild to test this?)

mquandalle avatar Mar 21 '16 20:03 mquandalle

@mquandalle that only prevents alerts from firing if the overlay is already visible. That code does nothing to determine the nature of the error.

Since meteor is almost always used behind a proxy server, a lot of devs (myself included) will have a proxy server on it from day one of a project. Detecting error type by error code will depend on those devs all setting up their proxies "correctly" which may or may not happen. However, effectively just solving the "disconnected, can't reconnect" situation so that it doesn't fire an error would resolve both!

lassombra avatar Mar 21 '16 21:03 lassombra

that only prevents alerts from firing if the overlay is already visible.

No there is two parts in this callback body. the if (err) { part correspond to the case where we weren’t able to get a response from the server, and the following lines (37 to 48) are the case where the server responded and we decide to display the overlay or not according to this server response.

So do you think that if the server is not responding, this package shouldn’t display any warning?

mquandalle avatar Mar 21 '16 21:03 mquandalle

I think it should be configurable

lassombra avatar Mar 21 '16 21:03 lassombra

I'm not sure if I might have this same problem, but I'm on a slow machine, and each time I rebuild I get the error overlay and desktop notification as soon as the server says

=> Client modified -- refreshing

and the client starts refreshing.

trusktr avatar Mar 30 '16 17:03 trusktr

@trusktr That’s probably a bug introduced in 1.5 by 75012a2f165b3d37847221597ac5a620119ed1f6. @stubailo I think it’s better to revert this commit and publish a patch release.

mquandalle avatar Mar 30 '16 17:03 mquandalle

@mquandalle added you as a maintainer:

➜  dev-error-overlay git:(master) meteor admin maintainers simple:dev-error-overlay --add mquandalle
Adding a maintainer to simple:dev-error-overlay...

The maintainers for simple:dev-error-overlay are:
simple
mquandalle

Also, published 1.5.1 with the reverted commit.

stubailo avatar Mar 30 '16 17:03 stubailo

Thank you @stubailo, but I don’t have access to this Github repo :-)

mquandalle avatar Mar 30 '16 17:03 mquandalle

What a surprise! I thought I added you. Well, I just did!

stubailo avatar Mar 30 '16 18:03 stubailo

I just encountered what I believe is a variation on this over a VPN last night. There was enough latency in my VPN connection that this package would timeout then succeed, and then reset the socket, which hadn't even connected yet. Repeatedly. This prevented me from even establishing the socket connection. I had to turn this package off in order to actually test over VPN

lassombra avatar Aug 19 '16 15:08 lassombra