search

quarkiverse / quarkus-quinoa

Quinoa is a Quarkus extension which eases the development, the build and serving of single page apps (built with NodeJS: React, Angular, …) alongside Quarkus . It is possible to use it with a Quarkus backend in a single project
Apache License 2.0
76 stars 35 forks source link

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

Open mschorsch opened 3 months ago

mschorsch commented 3 months ago

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 commented 3 months ago

Weird, this is tested: https://github.com/quarkiverse/quarkus-quinoa/blob/main/integration-tests/src/test/java/io/quarkiverse/quinoa/it/QuinoaRootPathTest.java#L52

ia3andy commented 3 months ago

@mschorsch are you using Quarkus REST or RESTEasy?

ia3andy commented 3 months ago

I just tested it and it seems to work: image

ia3andy commented 3 months ago

@mschorsch could you create a small reproducer?

mschorsch commented 3 months ago

@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 commented 3 months ago

@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 commented 3 months ago

@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 commented 3 months ago

@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 tohttp://localhost:8080/foo`
ia3andy commented 3 months ago

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

geoand commented 3 months ago

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

gsmet commented 3 months ago

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 https://github.com/quarkusio/quarkus/pull/39965 and see if it fixes it for you.

melloware commented 3 months ago

@all-contributors add @mschorsch for bug

allcontributors[bot] commented 3 months ago

@melloware

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

mschorsch commented 3 months ago

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 https://github.com/quarkusio/quarkus/pull/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 commented 3 months ago

I can confirm I am getting this error:

2024-04-09 14:51:12,886 ERROR [com.mel.qua.sup.ErrorMapper] (vert.x-eventloop-thread-2) Failed to handle request: jakarta.ws.rs.NotFoundException: HTTP 404 Not Found
        at org.jboss.resteasy.reactive.server.handlers.RestInitialHandler.handle(RestInitialHandler.java:67)
        at io.quarkus.resteasy.reactive.server.runtime.QuarkusResteasyReactiveRequestContext.invokeHandler(QuarkusResteasyReactiveRequestContext.java:121)
        at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:147)
        at org.jboss.resteasy.reactive.server.handlers.RestInitialHandler.beginProcessing(RestInitialHandler.java:48)
        at org.jboss.resteasy.reactive.server.vertx.ResteasyReactiveVertxHandler.handle(ResteasyReactiveVertxHandler.java:23)
        at org.jboss.resteasy.reactive.server.vertx.ResteasyReactiveVertxHandler.handle(ResteasyReactiveVertxHandler.java:10)
        at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1285)
        at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:177)
        at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:137)
        at io.quarkiverse.quinoa.QuinoaRecorder.next(QuinoaRecorder.java:91)
        at io.quarkiverse.quinoa.QuinoaDevProxyHandler.lambda$handleHttpRequest$0(QuinoaDevProxyHandler.java:105)
        at io.vertx.ext.web.client.impl.HttpContext.handleDispatchResponse(HttpContext.java:397)
        at io.vertx.ext.web.client.impl.HttpContext.execute(HttpContext.java:384)
        at io.vertx.ext.web.client.impl.HttpContext.next(HttpContext.java:362)
        at io.vertx.ext.web.client.impl.HttpContext.fire(HttpContext.java:329)
        at io.vertx.ext.web.client.impl.HttpContext.dispatchResponse(HttpContext.java:291)
        at io.vertx.ext.web.client.impl.HttpContext.lambda$null$7(HttpContext.java:507)
        at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:279)
        at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:261)
        at io.vertx.core.impl.ContextInternal.lambda$runOnContext$0(ContextInternal.java:59)
        at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:173)
        at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:166)
        at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:470)
        at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:566)
        at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
        at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
        at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
        at java.base/java.lang.Thread.run(Thread.java:833)

Clone this repo: https://github.com/melloware/quarkus-primereact

Run mvn quarkus:dev and hit localhost:8080 you will see the error.

melloware commented 3 months ago

Nevermind had something else running on port 3000

phillip-kruger commented 3 months ago

@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 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

I'll have a look. Give me a day

ia3andy commented 3 months ago

@phillip-kruger is working on a fix :)

ia3andy commented 3 months ago

@mschorsch thanks for reporting and investigating this!

mschorsch commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

Perfect :)

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

mschorsch commented 3 months ago

@ia3andy Should we close the issue?

melloware commented 3 months ago

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

UbiquitousBear commented 3 months ago

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.