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

Status returned by HasErrorParameter#setErrorParameter is ignored. #13421

Open
kyberorg opened this issue Apr 6, 2022 · 13 comments
Open

Status returned by HasErrorParameter#setErrorParameter is ignored. #13421

kyberorg opened this issue Apr 6, 2022 · 13 comments

Comments

@kyberorg
Copy link

kyberorg commented Apr 6, 2022

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

  • Download sample project from start.vaadin.com
  • Add RouteNotFoundView given at bug description.
  • Run project
  • visit any non-existent route

Versions

  • Vaadin / Flow version: 23.0.3
  • Java version: 17.0.3
  • OS version: Linux Manjaro, kernel 5.16.14-1-MANJARO
  • Browser version (if applicable): Firefox 98.0.2
  • Application Server (if applicable): the one goes with Spring Boot
  • IDE (if applicable): Intelij IDEA
@caalador caalador added bug hilla Issues related to Hilla labels Apr 6, 2022
@caalador
Copy link
Contributor

caalador commented Apr 6, 2022

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

kyberorg commented Apr 6, 2022

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

@darmro
Copy link

darmro commented May 20, 2022

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

@Legioth
Copy link
Member

Legioth commented Jun 22, 2022

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.

Artur- added a commit that referenced this issue Jun 22, 2022
Returns 404 for non existant views

Helps with #13421
@platosha
Copy link
Contributor

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 vaadin/docs#1521

taefi pushed a commit that referenced this issue Jun 27, 2022
* fix: Use correct view when using eagerServerLoad

Returns 404 for non-existant views

Helps with #13421
vaadin-bot pushed a commit that referenced this issue Jun 27, 2022
* fix: Use correct view when using eagerServerLoad

Returns 404 for non-existant views

Helps with #13421
vaadin-bot pushed a commit that referenced this issue Jun 27, 2022
* fix: Use correct view when using eagerServerLoad

Returns 404 for non-existant views

Helps with #13421
taefi pushed a commit that referenced this issue Jun 27, 2022
* fix: Use correct view when using eagerServerLoad

Returns 404 for non-existant views

Helps with #13421

(cherry picked from commit 87c220a)
mcollovati pushed a commit that referenced this issue Jun 27, 2022
taefi pushed a commit that referenced this issue Jun 27, 2022
…14076)

* fix: Use correct view when using eagerServerLoad (#14036)

* fix: Use correct view when using eagerServerLoad

Returns 404 for non-existant views

Helps with #13421

* Fix version

Co-authored-by: Artur <artur@vaadin.com>
taefi added a commit that referenced this issue Jun 28, 2022
…14078)

* fix: Use correct view when using eagerServerLoad (#14036)

Returns 404 for non-existent views

Helps with #13421

(cherry picked from commit 87c220a)
@paulroemer
Copy link
Contributor

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

darmro commented Jul 9, 2022

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

darmro commented Jul 14, 2022

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 afterNavigation is 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
Copy link
Contributor

knoobie commented Jan 25, 2023

@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
Copy link
Contributor

mshabarov commented Jan 25, 2023

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

@alexanoid
Copy link

@Artur-
Copy link
Member

Artur- commented Apr 1, 2023

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

@mcollovati
Copy link
Collaborator

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

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

No branches or pull requests