vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
618 stars 167 forks source link

Status returned by HasErrorParameter#setErrorParameter is ignored. #13421

Open kyberorg opened 2 years ago

kyberorg commented 2 years ago

Description of the bug

I just downloaded sample project from start.vaadin.com and added RouteNotFoundView almost same as in documentation.

public class RouteNotFoundView extends Div implements HasErrorParameter<NotFoundException> {
    @Override
    public int setErrorParameter(BeforeEnterEvent event, ErrorParameter<NotFoundException> parameter) {
        getElement().setText("Cannot not navigate to '"
                + event.getLocation().getPath()
                + "'");
        return HttpServletResponse.SC_NOT_FOUND;
       }
    }

But it still returns HTTP status 200 and ignores HttpServletResponse.SC_NOT_FOUND

$ wget http://localhost:8080/notFound                                                                                                                        
--2022-04-05 14:13:51--  http://localhost:8080/notFound
Resolving localhost (localhost)... ::1, 127.0.0.1
Connecting to localhost (localhost)|::1|:8080... connected.
HTTP request sent, awaiting response... 200 
Length: 3665 (3,6K) [text/html]
Saving to: ‘notFound’

notFound                                        100%[====================================================================================================>]   3,58K  --.-KB/s    in 0s      

2022-04-05 14:13:51 (344 MB/s) - ‘notFound’ saved [3665/3665]

Expected behavior

Status 404 or any other status, which set as return value should be returned instead.

Minimal reproducible example

Versions

caalador commented 2 years ago

running the application with vaadin.useDeprecatedV14Bootstrapping=true You get

HTTP/1.1 404
Set-Cookie: JSESSIONID=96321ACDFA9EB15253FB7B639D3ACEC5; Path=/; HttpOnly
Cache-Control: no-cache, no-store
Pragma: no-cache
Expires: Thu, 01 Jan 1970 00:00:00 GMT
Content-Type: text/html;charset=utf-8
Transfer-Encoding: chunked
Date: Wed, 06 Apr 2022 09:32:20 GMT

So it is the js bootstrap that does a 200 response and later receives the 404 response for the non existent route.

kyberorg commented 2 years ago

@caalador thanks for advice. I confirm that your workaround works.

darmro commented 2 years ago

Hi, I can confirm that the same issue is for Vaadin 22.

Legioth commented 2 years ago

The reason for this behaviour is that with the integration between Flow and Hilla, the server cannot know that there won't be a client-side route that would handle the same URL and it will therefore not even try to resolve server-side routes while generating the initial HTML response. The problem would thus not be that returning HttpServletResponse.SC_NOT_FOUND is ignored, but rather that the code that returns HttpServletResponse.SC_NOT_FOUND isn't even run before the initial HTTP response is written.

I haven't tried this in practice, but I believe you can override this behaviour by setting the eagerServerLoad configuration parameter to true.

platosha commented 2 years ago

Apart from the bug addressed in #14036, the documentation is currently misleading here to not mention a different behavior with and without eagerServerLoad enabled. See https://github.com/vaadin/docs/issues/1521

paulroemer commented 2 years ago

I can confirm that with 23.1.3 eagerServerLoad works as expected and the application behaves as it should again. As Leif already mentioned above, this only works for applications that only use server-side routes.

For SpringBoot apps just add

vaadin.eagerServerLoad=true
darmro commented 2 years ago

Unfortunately, there is one problem with eagerServerLoad=true. If set, on page load the content of the view is at first placed directly into the body of the HTML (directly, not wrapped in <flow-container-web-xxx>) and then after some time moved inside <div id = "outlet"> (and wrapped in ). This causes that when the main div of the view is position = relative (I need it because on the left side I have a separate menu bar that does not overlap the view), at first the content of the view is shown in the browser from the middle of the page, and after some while properly. error-1

Effect is ugly for the user: error2

If eagerServerLoad is not set then the content of the view is also first placed directly in the body, but wrapped in <flow-container-web-xxx>, which is not relative, so everything is displayed ok for the user. After some time view content is moved inside <div id = "outlet"> of course. Effect: error1

darmro commented 2 years ago

Second problem with eagerServerLoad. If set, the afterNavigation for the view is called 2 times! If not set, then once (as it should be). Why it is called twice with eagerServerLoad=true? I have spring boot, java only, Vaadin 22.0.18 application. The fact that afterNavigationis called twice is a big inconvenience for me, because this is where my views are often built, data is downloaded, and the modal window is sometimes opened (depending on the parameters). So with eagerServerLoad=true I would have to somehow handle it all to run once.

knoobie commented 1 year ago

@mshabarov FYI: the mentioned workaround to use "vaadin.useDeprecatedV14Bootstrapping=true" has caused multiple reports on discord and stack overflow that it renders the UI twice (see comments above as well) - this should either documented or fixed.. because people run into this when migrating from V14

mshabarov commented 1 year ago

@knoobie thanks for a reminder, Flow team will take a look how it can be fixed.

alexanoid commented 1 year ago

Hi! I also faced the "twice rendering" issue https://stackoverflow.com/questions/75166305/vaadin23-vaadin-eagerserverload-true-and-beforeenterobserver

Artur- commented 1 year ago

Some fundamental bugs in eager server load should be fixed by #16498

mcollovati commented 10 months ago

All reported issues with vaadin.eagerServerLoad do not reproduce anymore with Vaadin 24.

16498 has not been back-ported to Vaadin 23, so double initialization still happens