(Some) client-side problems are not properly handled according to SystemMessages set by the user

enver-haase commented 5 months ago

Description of the bug

When Vaadin was struck by the Atmosphere bug Bug with 1|X problem then the users would see Invalid JSON from server: 1|X on the front-end with the application becoming non-navigable.

Which clearly documented that Vaadin does not in all cases handle client-side errors properly, with properly meaning that Vaadin's SystemMessages should be honoured.

In the example case above a customer set the messages to be translated and also that upon any error, Vaadin should redirect to the starting page (an error URL). That did not happen.

There are likely more instances of client-side problems not properly handled.

Expected behavior

At least around any 3rd party code usage (like Atmosphere code) we should have an error/exception handler that honours the customer settings in SystemMessages.

Minimal reproducible example

Well, maybe go back to a Vaadin version that imports the broken Atmosphere version.

Vaadin staff can read about details here: https://vaadin.slack.com/archives/C6X43FE8M/p1708507843759869

The only mention of "X" I find in Atmosphere is the heartbeat mechanism https://github.com/Atmosphere/atmosphere/blob/24a1456c137e36dfe7c7a6b180ebe299713f[…]/main/java/org/atmosphere/interceptor/HeartbeatInterceptor.java

That is supposed to be caught by atmosphere JS and not passed on to Vaadin

But heck, if it IS passed on to Vaadin we need to guard our code. And that's a general concern and not specific to Atmosphere calls.

Versions

Vaadin / Flow version: 24.3.9
Java version: n/a
OS version: n/a
Browser version (if applicable): n/a
Application Server (if applicable): n/a
IDE (if applicable): n/a

subITCSS commented 5 months ago

So for example when having this SystemMessages defined - we get messages like this shown up.

public class CSystemMessages extends CustomizedSystemMessages {

    public static final String URL = AuthenticationModule.DEFAULT_AUTH_ROUTE;

    public CSystemMessages() {
        boolean showNotification = SystemInfo.getShowSystemNotifications();
        setInternalErrorNotificationEnabled(showNotification);
        setSessionExpiredNotificationEnabled(showNotification);
        setCookiesDisabledNotificationEnabled(showNotification);
        setInternalErrorURL(URL);
        setSessionExpiredURL(URL);
        setCookiesDisabledURL(URL);
    }
}

enver-haase commented 4 months ago

There is another example other than the 3rd party Atmosphere exception sketched above (Atmosphere guys have corrected the library but our handling of the error, should it ever occur again, is sub-par as you can see above).

Here is the other example: VaadinSession.java has checkHasLock(message) with a message that is not translated by SystemMessages and also when in production Mode and assertions are turned on, the code JVM will throw AssertionError and there is no SystemMessages handling of the error like re-starting the application (or is there?).

vaadin / flow