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

SPA-Routing no longer works properly since Quarkus 3.9 #666

Open
mschorsch opened this issue Apr 8, 2024 · 32 comments
Open

SPA-Routing no longer works properly since Quarkus 3.9 #666

mschorsch opened this issue Apr 8, 2024 · 32 comments
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@mschorsch
Copy link

mschorsch commented Apr 8, 2024

Describe the bug

We use SPA routing in some of our applications. Since Quarkus 3.9, SPA routing no longer works correctly if the URL is entered directly.

Example: Suppose we have a Quarkus application with a React frontend (the routing in the frontend works via react-router). All Resteasy Reactive REST-API endpoints are located under /api/. If the application is called e.g. http://localhost:8080/ then the SPA routing works correctly. However, if e.g. http://localhost:8080/page1 is entered directly in the browser, we get a 404 instead of the index.html since Quarkus 3.9. Quarkus 3.8.x works as expected.

Does this possibly have something to do with the renaming of the extensions in Quarkus 3.9?

Quinoa version

2.3.6

Quarkus version

3.9.2

Build / Runtime

Create React App (CRA)

Package Manager

YARN

Steps to reproduce the behavior

No response

Expected behavior

No response

@ia3andy
Copy link
Collaborator

ia3andy commented Apr 8, 2024

@mschorsch are you using Quarkus REST or RESTEasy?

@ia3andy
Copy link
Collaborator

ia3andy commented Apr 8, 2024

I just tested it and it seems to work:
image

@ia3andy
Copy link
Collaborator

ia3andy commented Apr 8, 2024

@mschorsch could you create a small reproducer?

@mschorsch
Copy link
Author

@mschorsch are you using Quarkus REST or RESTEasy?

We use Resteasy Reactive (Quarkus REST).

I just tested it and it seems to work:

Mmm. Just to be sure, did you enter the URL http://localhost:8080/foo directly or did you navigate there from http://localhost:8080/? The error only occurs for us if the URL http://localhost:8080/foo is entered directly in the browser.

@mschorsch could you create a small reproducer?

We are trying to create one.

@ia3andy
Copy link
Collaborator

ia3andy commented Apr 8, 2024

@mschorsch are you using Quarkus REST or RESTEasy?

We use Resteasy Reactive (Quarkus REST).

I just tested it and it seems to work:

Mmm. Just to be sure, did you enter the URL http://localhost:8080/foo directly or did you navigate there from http://localhost:8080/? The error only occurs for us if the URL http://localhost:8080/foo is entered directly in the browser.

In the browser

@mschorsch could you create a small reproducer?

We are trying to create one.

🙏

@mschorsch
Copy link
Author

mschorsch commented Apr 9, 2024

@ia3andy We don't have a reproducer yet, but we have found the problem.

If an endpoint is not found, a NotFoundException is thrown. This exception is caught by a custom ExceptionMapper. Since Quarkus 3.9 this means that the NotFoundException is handled by the ExceptionMapper and not by Quinoa.

The problem should therefore be easy to reproduce by defining a global ExceptionMapper.

@mschorsch
Copy link
Author

@ia3andy The problem can be reproduced by using the integrations-tests from quarkus-quinoa.

  1. Change the Quarkus version to 3.9.2
  2. Enable spa-routing in the application.properties
  3. Add the following ExceptionMapper
public class ExceptionMappers {

    @ServerExceptionMapper
    public Response handleExceptions(WebApplicationException exception) {
        return exception.getResponse();
    }
}
  1. Start mvn -Dquarkus.profile=react quarkus:dev
  2. Navigate to http://localhost:8080/foo`

@ia3andy
Copy link
Collaborator

ia3andy commented Apr 9, 2024

@phillip-kruger @geoand did we change something in Quarkus for this?

@geoand
Copy link
Contributor

geoand commented Apr 9, 2024

It might be related to the Route on 404 thing that Philip changed

@gsmet
Copy link
Member

gsmet commented Apr 9, 2024

Yeah, I would advise to wait for 3.9.3 that I will release tomorrow as it might contain a fix for it.

@mschorsch You can try building quarkusio/quarkus#39965 and see if it fixes it for you.

@melloware melloware added the bug Something isn't working label Apr 9, 2024
@melloware
Copy link
Contributor

@all-contributors add @mschorsch for bug

Copy link
Contributor

@melloware

I've put up a pull request to add @mschorsch! 🎉

@mschorsch
Copy link
Author

Yeah, I would advise to wait for 3.9.3 that I will release tomorrow as it might contain a fix for it.

@mschorsch You can try building quarkusio/quarkus#39965 and see if it fixes it for you.

Thanks @gsmet. I've build quarkusio/quarkus#39965 an tested against quarkus-quinoa, it seems that 3.6.3 doesn't fix the issue. Furthermore, I do not see any pull request that could resolve the issue.

Maybe I did something wrong or overlooked something :). Let's wait for Quarkus 3.6.3.

@melloware

This comment was marked as outdated.

@melloware

This comment has been minimized.

@phillip-kruger
Copy link
Member

@ia3andy We don't have a reproducer yet, but we have found the problem.

If an endpoint is not found, a NotFoundException is thrown. This exception is caught by a custom ExceptionMapper. Since Quarkus 3.9 this means that the NotFoundException is handled by the ExceptionMapper and not by Quinoa.

The problem should therefore be easy to reproduce by defining a global ExceptionMapper.

This seems like the correct behavior ? Or am I missing something ?

@mschorsch
Copy link
Author

@phillip-kruger If all REST endpoints are under /api, for example, the expected behavior is that when SPA routing is activated, calling /foo returns the index.html with HTTP status 200. If a non-existent REST endpoint /api/foo is called, then the ExceptionMapper should be called or, if not available, a standard HTTP status 404 should be returned. Since Quarkus 3.9, however, the ExceptionMappers are also executed when /foo is called.

@ia3andy
Copy link
Collaborator

ia3andy commented Apr 10, 2024

We found the issue, As soon as a ExceptionMapper is set, REST now assume that it should deal with 404. I think we should check the ResumeOn404 option (BuildItem and Config) before throwing the 404.

@phillip-kruger
Copy link
Member

I'll have a look. Give me a day

@ia3andy
Copy link
Collaborator

ia3andy commented Apr 10, 2024

@phillip-kruger is working on a fix :)

@ia3andy
Copy link
Collaborator

ia3andy commented Apr 10, 2024

@mschorsch thanks for reporting and investigating this!

@mschorsch
Copy link
Author

As a workaround: Define a custom SPA routing handler (e.g. https://docs.quarkiverse.io/quarkus-quinoa/dev/advanced-guides.html#spa-routing).

This works as expected.

@melloware
Copy link
Contributor

@mschorsch nice! Yeah I use RestEasy Classic so I have been using that SPA class so I guess that is why I didn't see the issues!

@phillip-kruger
Copy link
Member

phillip-kruger commented Apr 14, 2024

Hi all.

So I think the current way to handle this (3.9) is correct, and pre-3.9 was wrong. To see the details for this, go to https://github.com/phillip-kruger/quinoa-and-rest/

tldr:

Using Quarkus correctly, when using more than one technology on HTTP, you should separate those under different roots:

Typically, in this example, you would serve Web content on / and REST on /api. (by using config: quarkus.rest.path=/api)

The pre-3.9 breaks this when using like you want to (Having SPA and a Exception Handler) as even REST endpoint will redirect to index.html.

Using Quarkus "incorrectly", i.e sharing both REST and Web on / still behave as above (i.e REST Endpoints redirect to index.html) but one can argue that is what the user wants.

If you want to use the same root (/) but want REST to use the exception handler, add this to your exception mapper:

@Context
private UriInfo uriInfo;

@ServerExceptionMapper
public Response handleWeb404(NotFoundException exception) {
    String path = uriInfo.getRequestUri().getPath();
    if(path.startsWith("/api")){
        return handleExceptions(exception);
    }
    return Response.seeOther(URI.create("/")).build();
}

I hope this help. I'll work with @ia3andy to see if he really want to support the old way, that we add support for that in Quinoa without breaking REST Endpoints.

@mschorsch
Copy link
Author

mschorsch commented Apr 15, 2024

@phillip-kruger Thanks for the detailed analysis.

If I want to deploy the web content under / and REST under /api then I just need to set quarkus.rest.path=/api in the application.properties and everything works as expected, right?

This should be documented.

@phillip-kruger
Copy link
Member

phillip-kruger commented Apr 15, 2024

Yes, and your REST code with the annotations (@Path) will not include the /api.

So @Path("/api/foo") becomes @Path("/foo")
Yes I agree, I think this should be in the Quinoa docs

@mschorsch
Copy link
Author

Perfect :)

I can confirm that by setting quarkus.rest.path everything works as expected.

@mschorsch
Copy link
Author

@ia3andy Should we close the issue?

@melloware melloware added documentation Improvements or additions to documentation and removed bug Something isn't working labels Apr 18, 2024
@melloware
Copy link
Contributor

I changed it to a documentation ticket. I think what @phillip-kruger elaborated on should be somehow added to the docs.

@UbiquitousBear
Copy link

I have a potentially related (or maybe not) issue here with dynamic routing in nextjs:

I've using the graphql extension which exposes /graphql, also with quarkus.quinoa.enable-spa-routing=true. My issue is that when I go to http://localhost:8080/components/[id], the contents of http://localhost:8080/ (ie src/pages/index.tsx is shown. If it try the same request over the nextjs dev server directly, ie http://localhost:3000/components/[id] the routing works as expected.

Inspecting the state of next/router shows that the path is picked up, but nothing else is. Unsure whether this is a next issue or a quinoa issue.

@egore
Copy link

egore commented Dec 9, 2024

Setting quarkus.rest.path stopped working when upgrading from quarkus 3.15.1 to 3.16.0, currently rendering SPA-routing non-working. It still works in the development mode, but not in the built artifact (in my case a docker container). So far I haven't identified a solution, and only staying at quarkus 3.15.x seems to be working (with quinoa 2.5.0).

I bisected this in a project of mine (https://gitlab.com/appframework/tasker-web/-/commit/560ad663352ab6f28b54492758184e30ec08371f)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

8 participants