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

Exceptions in a LitRenderer causes "Internal error" #14091

Open
Artur- opened this issue Jun 28, 2022 · 8 comments
Open

Exceptions in a LitRenderer causes "Internal error" #14091

Artur- opened this issue Jun 28, 2022 · 8 comments

Comments

@Artur-
Copy link
Member

Artur- commented Jun 28, 2022

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

@mcollovati
Copy link
Collaborator

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.

@caalador
Copy link
Contributor

caalador commented Aug 28, 2023

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

@koen-serneels
Copy link

koen-serneels commented Sep 8, 2023

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

@caalador
Copy link
Contributor

caalador commented Sep 8, 2023

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.

@mshabarov
Copy link
Contributor

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

@mvysny
Copy link
Member

mvysny commented Feb 22, 2024

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
Copy link
Member

mvysny commented Feb 22, 2024

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
Copy link
Member

mvysny commented Feb 22, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🔖 Normal Priority (P2)
Development

No branches or pull requests

6 participants