Custom ErrorHandler implementation's UI changes are ignored

[padisah]

Description of the bug

We have created a customer error handler by extending the DefaultErrorHandler class.
This is added to the session, and it would create a Dialog (or later a Notification for testing).

The feature works when the exception occurs during component constructor, in general when it is a navigation to a given target.
The feature doesn't work, when the exception occurs in the lambda passed to a grid.addComponent method.

I debugged what happens, in the non-working case the exception is caught at VaadinService.handleExceptionDuringRequest

here the set errorHandler is called on line 1586

            if (vaadinSession != null) {
                vaadinSession.getErrorHandler().error(new ErrorEvent(t));
            }

During the execution everything looks ok, there is a UI instance, the ui component is created, and even attached to the ui. Nothing is visible in the browser.

on line 1594 writeUncachedStringResponse is called with the system internal error message, and it is set as the response.
Checking it in the browser, the response looks like:

for(;;);[
    {
        "changes": {},
        "resources": {},
        "locales": {},
        "meta": {
            "appError": {
                "caption": "Internal error",
                "url": null,
                "message": "Please notify the administrator. Take note of any unsaved data, and click here or press ESC to continue.",
                "details": null,
                "querySelector": null
            }
        },
        "syncId": -1
    }
]

I think the reason is, that in the above message the Vaadin server does not send the ui changes requested in the errorHandler, not even if it is wrapped in a UI.access call. These tests were running with a single browser tab open for the server, so it cannot swap the ui instance to another one.

The online documentatation does state, that the errorHandler can add a notification, or other ui changes.

https://vaadin.com/docs/latest/advanced/custom-error-handler

Expected behavior

The requested ui changes should appear in the browser, for example Dialog open.

Minimal reproducible example

public class TestExceptionHandler extends DefaultErrorHandler {

@Override
  public void error(final ErrorEvent event) {
    Command c = () -> Notification.show("message").addThemeVariants(NotificationVariant.LUMO_ERROR);
    Optional.ofNullable(UI.getCurrent()).ifPresent((UI ui) -> ui.accessSynchronously(c));
  }
}
....
public class TestGrid extends TreeGrid<TestDto> {

       public TestGrid() {

        addComponentColumn(this::getTestColumn)
                .setHeader("Test Column")
                .setSortProperty("test);
}

    private Component getTestColumn(TestDto testDto {
        throw new NullPointerException(); // throws an exception from a columnGenerator
 }
}

Versions

• Vaadin / Flow version: 23.3.16
• Java version: 17
• OS version: Linux
• Browser version (if applicable): n/a
• Application Server (if applicable): embedded jetty
• IDE (if applicable): n/a

[mcollovati]

It seems similar to #14091

[padisah]

I could debug the ErrorHandler being executed. But anything that it would change in the browser is ignored, so server side action, like logging or writing into db would work.

In the handleExceptionDuringRequest code, first it runs the ErrorHandler, then if this is a UIDL request, goes to a branch that responds only with the Internal error system message. I would also mention, that we don't want to see the internal error message, after we have already customized the error behaviour.

For this particular case, even a workaround would be a solution.

[oliveryasuna]

All of the following examples reproduce the problem. The error handler is called, but no UI changes in the error method are reflected to the client.

grid.addColumn(person -> {
      throw new AnonException();
    })
    .setHeader("Name");

grid.addComponentColumn(person -> {
      throw new AnonException();
    })
    .setHeader("Name");

grid.addColumn(LitRenderer
        .<Person>of("<b>${item.name}</b>")
        .withProperty("name", person -> {
          throw new AnonException();
        }))
    .setHeader("Name");

I imagine there are other cases where an exception would trigger the problem.

[oliveryasuna]

Let's take a look at VaadinService#handleExceptionDuringRequest(...) (full method at bottom) and dissect the issue:

vaadinSession.getErrorHandler().error(new ErrorEvent(t)) triggers the error handler, whether it be the default or custom.

Say we have a custom error handler that modifies the UI. For example, Notification.show("Uh oh, spaghetti-o"). The UI changes are queued for the next normal UIDL message to the client.

However, no normal UIDL message is sent, as if we continue reading the method, we see that a "critical notification" message is sent.

See below the following code snippet (original method) for a revised method:

private void handleExceptionDuringRequest(VaadinRequest request,
        VaadinResponse response, VaadinSession vaadinSession, Exception t)
        throws ServiceException {
    if (vaadinSession != null) {
        vaadinSession.lock();
    }
    try {
        if (vaadinSession != null) {
            vaadinSession.getErrorHandler().error(new ErrorEvent(t));
        }
        // if this was an UIDL request, send UIDL back to the client
        if (HandlerHelper.isRequestType(request, RequestType.UIDL)) {
            SystemMessages ci = getSystemMessages(
                    HandlerHelper.findLocale(vaadinSession, request),
                    request);
            try {
                writeUncachedStringResponse(response,
                        JsonConstants.JSON_CONTENT_TYPE,
                        createCriticalNotificationJSON(
                                ci.getInternalErrorCaption(),
                                ci.getInternalErrorMessage(), null,
                                ci.getInternalErrorURL()));
            } catch (IOException e) {
                // An exception occurred while writing the response. Log
                // it and continue handling only the original error.
                getLogger().warn(
                        "Failed to write critical notification response to the client",
                        e);
            }
        } else {
            // Re-throw other exceptions
            throw new ServiceException(t);
        }
    } finally {
        if (vaadinSession != null) {
            vaadinSession.unlock();
        }
    }

}

We could instead write a UIDL. This is definitely not a fix, just a workaround.

private void handleExceptionDuringRequest(VaadinRequest request,
                                          VaadinResponse response, VaadinSession vaadinSession, Exception t)
    throws ServiceException {
  if(vaadinSession != null) {
    vaadinSession.lock();
  }
  try {
    if(vaadinSession != null) {
      vaadinSession.getErrorHandler().error(new ErrorEvent(t));
    }
    // if this was an UIDL request, send UIDL back to the client
    if(HandlerHelper.isRequestType(request, RequestType.UIDL)) {
      getRequestHandlers().forEach(requestHandler -> {
        if(requestHandler instanceof UidlRequestHandler) {
          try {
            requestHandler.handleRequest(vaadinSession, request, response);
          } catch(final IOException e) {
            getLogger().warn("Failed to pass UI to client.", e);
          }
        }
      });
    } else {
      // Re-throw other exceptions
      throw new ServiceException(t);
    }
  } finally {
    if(vaadinSession != null) {
      vaadinSession.unlock();
    }
  }

}

[mcollovati]

The issue was triaged and currently added to the backlog priority queue for further investigation

[TatuLund]

It has been long since I have investigated this kind of cases before, but I have some example codes from the past.

One approach I found working well is to set the error handler in UI init listener.

https://github.com/TatuLund/devday-demo-flow/blob/master/src/main/java/com/vaadin/devday/demo/InitListener.java#L59

Then I have proper UI reference to the latest created UI and can use ui.access(..).

This was related to another ticket #10533, which includes the same code #10533 (comment)

[oliveryasuna]

@TatuLund Adding the error handler in the UI init listener does not fix this problem.

[ankeshKapil]

The bug is real, ErrorHandler does get called but UI updates like Notifications are lost. The application just shows Vaadin's internal error notification.

[caalador]

Initial issue that I have now found is that the errorHandler should work as noted, but not in the case that the error happens in an BeforeClientResponse which happens after the RPCHandler.

So this is basically an exception that breaks the writing of the UIDL response and thus returns only the internal error data.

So as an example if I update the button test for custom ErrorHandler from

e -> {
  throw new IllegalStateException("Intentional fail in click handler");
}

which adds a div to

e -> {
  e.getSource().getUI().get().getInternals().getStateTree().beforeClientResponse(
    getElement().getNode(), 
      executionContext -> {
        throw new IllegalStateException("Intentional fail in click handler");
      }
  );
}

The div doesn't get added anymore and there is a internal error notification.

[vaadin-bot]

This ticket/PR has been released with Vaadin 24.2.0.alpha10 and is also targeting the upcoming stable 24.2.0 version.

[vaadin-bot]

This ticket/PR has been released with Vaadin 24.1.8.

[vaadin-bot]

This ticket/PR has been released with Vaadin 24.0.14.