flow icon indicating copy to clipboard operation
flow copied to clipboard

Exceptions in a LitRenderer causes "Internal error"

Open Artur- opened this issue 2 years ago • 8 comments

Description of the bug

        Grid<SampleBook> grid = new Grid<>(SampleBook.class);
        LitRenderer<SampleBook> imageRenderer = LitRenderer
                .<SampleBook>of("foo").withProperty("foo",
                        item -> {
                            throw new RuntimeException("Oh no");
                        });
        grid.addColumn(imageRenderer);
        grid.setItems(new SampleBook());

causes

Internal error
Please notify the administrator. Take note of any unsaved data, and click here or press ESC to continue.

Expected behavior

The exception is passed to the error handler

Minimal reproducible example

See above

Versions

Vaadin: 23.1.2 Flow: 23.1.2 Java: Homebrew 11.0.12 OS: aarch64 Mac OS X 12.4 Browser: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/102.0.0.0 Safari/537.36

Artur- avatar Jun 28 '22 11:06 Artur-

The issue is not limited to the LitRender, but can happen with all kind of renderers. For example adding a column with a ValueProvider that throws an exception (so using ColumnPathRenderer under the hood) will lead to the same problem.

mcollovati avatar Jul 12 '23 16:07 mcollovati

So is the expectation that the exception would be shown in the internal error dialog?

caalador avatar Aug 28 '23 07:08 caalador

Hi. Is there any progress on this issue? This is a problem for us as exceptions that happen in for example lambda´s for column renders do not trigger our standard error screen and worse, they are also not logged. This is all that happens in those cases which is really not acceptable:

image

koen-serneels avatar Sep 08 '23 06:09 koen-serneels

There is a change for 24.0.14, 24.1.8 and 24.2.0.alpha10 that run the custom ErrorHandler so it can update the UI and does not throw the internal error if there is a custom error handler set. Else it uses the default which should log the exception and send only internal error to the client.

caalador avatar Sep 08 '23 07:09 caalador

Cherry picked into Flow 23.3. Wait for releases, estimation - this week.

mshabarov avatar Sep 11 '23 11:09 mshabarov

Yup, can be reproduced easily with:

@Route("")
public class MainView extends VerticalLayout {

    public MainView() {
        final Grid<Integer> grid = new Grid<>(Integer.class);
        add(grid);
        grid.addColumn(new ComponentRenderer<>((SerializableSupplier<Component>) () -> {
            throw new RuntimeException("Ha!");
        })).setHeader("Ha");
        add(new Button("Click", e -> grid.setItems(IntStream.range(0, 10).boxed().toList())));
    }
}

In general, I think this kind of response is received whenever exception is thrown when the response UIDL is being written.

Since the UIDL is written to JsonObject in UidlRequestHandler.writeUidl(), it has not yet been sent to the browser. I wonder whether there's a way to call Session's ErrorHandler and then write changes performed by ErrorHandler only. But then again, how can Vaadin possibly know which changes are "good" and needs to be kept, and which changes are "bad" and need to be thrown away? Probably there is no way of telling and that's why Vaadin sends a full-resync request to the browser.

@caalador you mentioned a change already merged into Vaadin 24.2.0. However the bug is reproducible in Vaadin 24.3.5, so I wonder whether the PR landed?

EDIT: wait, the DefaultErrorHandler seems to be notified!

2024-02-22 14:13:53.845 [qtp1484171695-36] ERROR com.vaadin.flow.server.DefaultErrorHandler - 
java.lang.RuntimeException: Ha!
	at com.vaadin.starter.skeleton.MainView$1.get(MainView.java:29)
	at com.vaadin.starter.skeleton.MainView$1.get(MainView.java:26)
	at com.vaadin.flow.data.renderer.ComponentRenderer.createComponent(ComponentRenderer.java:222)

Interesting. The exception is caught in StateTree.runExecutionsBeforeClientResponse() and checks whether the error handler is the default one.

  • When the error handler is DefaultErrorHandler then the exception bubbles out of RequestHandler.handleRequest() which causes VaadinService to call handleExceptionDuringRequest() which then calls the ErrorHandler and displays the "Internal Error, please notify your administrator"
  • On the other hand if the error handler is not DefaultErrorHandler then the StateTree.runExecutionsBeforeClientResponse() calls error handler and completes normally, which then allows the error handler to e.g. draw a Dialog or such.

I wonder what is the reason for this kind of dichotomy in behavior: either the exception is Internal Error or it isn't, regardless of the ErrorHandler currently configured, and therefore the response should be the same.

mvysny avatar Feb 22 '24 12:02 mvysny

It's also possible to achieve "Internal error" even with custom error handler set: when the ErrorHandler itself throws an exception, then it bubbles out and reaches handleExceptionDuringRequest() and the "Internal Error" is shown.

mvysny avatar Feb 22 '24 12:02 mvysny

Regardless, this problem seems to be fixed, at least in Vaadin 24.3.5 and 23.3.33 - the custom ErrorHandler is notified and the "Internal Error" div is not shown. I propose to close this ticket as fixed.

mvysny avatar Feb 22 '24 13:02 mvysny