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

Default reactive route returns 404 #38840

Closed
geromueller opened this issue Feb 18, 2024 · 26 comments · Fixed by #38852
Closed

Default reactive route returns 404 #38840

geromueller opened this issue Feb 18, 2024 · 26 comments · Fixed by #38852
Assignees
Labels
area/vertx kind/bug Something isn't working
Milestone

Comments

@geromueller
Copy link

Describe the bug

For single page apps unknown routes should return index.html. In classic mode it is possible to add a "last" route which redirects to index.html. In reactive mode the default route returns 404, which breaks the router chain.

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@geromueller geromueller added the kind/bug Something isn't working label Feb 18, 2024
@geoand
Copy link
Contributor

geoand commented Feb 19, 2024

In classic mode it is possible to add a "last" route which redirects to index.html. In reactive mode the default route returns 404, which breaks the router chain

I am not sure what you mean by this.

Can you clarify please?

Thanks

@geoand geoand added the triage/needs-feedback We are waiting for feedback. label Feb 19, 2024
@gmuellerinform
Copy link
Contributor

gmuellerinform commented Feb 19, 2024

I was in a hurry yesterday, sorry!

In order to have a small and leightweight service, I deliver the frontend using Quarkus. This is how i make sure the single page app is loaded correctly in classic mode:

   void init(@Observes Router router) {
        router.getWithRegex("/.*").last().handler(rc -> {
            if (rc.normalizedPath().equals("index.hmtl"))
                rc.fail(404);
            else
                rc.reroute("index.hmtl");
        });
    }

But in reactive mode, this route never matches!

@geoand
Copy link
Contributor

geoand commented Feb 19, 2024

Thanks for the update, it's very helpful, but I still don't fully understand what But in reactive mode, this route never matches means.
Mind providing an example for this as well?

@gmuellerinform
Copy link
Contributor

gmuellerinform commented Feb 19, 2024

It is the same code, but some route/handler which is executed before calls the 404 instead of rc.next(), so the list of routes is not processed to the end.
But, when I move my route up in the order, just before the build in routes (order=10000), I have to guess if it is an SPA route or some actual resource / static file. My workaround curently looks like this:

void init(@Observes io.vertx.mutiny.ext.web.Router router) {
   router.getWithRegex("^(?!/assets\|/api\|/index\\.html).*").handler(rc -> {
      rc.reroute(reroutePath);
   });
}

@gmuellerinform
Copy link
Contributor

See here: https://stackoverflow.com/questions/66965460/quarkus-how-to-implement-routing-for-single-page-apps
I was actually surprised that quarkus does not support this out of the box...

@geoand
Copy link
Contributor

geoand commented Feb 19, 2024

Gotcha, thanks!

I'll leave it to @cescoffier. Also @ia3andy might have some ideas.

@ia3andy
Copy link
Contributor

ia3andy commented Feb 19, 2024

@gmuellerinform
Copy link
Contributor

@ia3andy I had a look at it, but is solved things I do not need (building frontend, proxies etc)

@gmuellerinform
Copy link
Contributor

gmuellerinform commented Feb 19, 2024

@geoand
Copy link
Contributor

geoand commented Feb 19, 2024

That is RESTEasy Reactive specific, it has no effect when using routes directly

@gmuellerinform
Copy link
Contributor

@geoand Are you sure? It actually says it does change routing behavior...

@geoand
Copy link
Contributor

geoand commented Feb 19, 2024

I am

@gmuellerinform
Copy link
Contributor

But why does quinoa have to use it then to allow SPA routing?

@geoand
Copy link
Contributor

geoand commented Feb 19, 2024

The reason it says that is because RESTEasy Reactive itself executes (from a Vertx perspective at least) as one big complicated route.
Normally when RESTEasy Reactive can't match a URL it ends the request with 404.

The build item you see allows the next route in the chain to continue the processing when RESTEasy Reactive has not matched the URL

@gmuellerinform
Copy link
Contributor

I mean, yes it is RESTEasy Reactive specific, but then this needs to be changed ;-)

@ia3andy
Copy link
Contributor

ia3andy commented Feb 19, 2024

I think there is an issue to make this easier, let me find it..

@gmuellerinform
Copy link
Contributor

Could you add an build time option for this, instead if having to create an extension?

@ia3andy
Copy link
Contributor

ia3andy commented Feb 19, 2024

@phillip-kruger didn't we talk about changing the 404 handling in dev mode to use the dev-ui instead or something? I can't find any issue about it?

@phillip-kruger
Copy link
Member

Yea we did, but nothing has been done yet. I have a branch with the index.html page now using Dev UI, I think that is a start.

@ia3andy
Copy link
Contributor

ia3andy commented Feb 19, 2024

AFAIK the problem is that the 404 handling is done by RR (and RC) with the same routing priority as any other RR resources which is not allowing any other service/extension to provide their own routes with a lower priority than RR.

The 404 handling should be dealt with at a lower priority to allow anyone to hook in. Still if it's not the case yet, I am pretty sure their are reasons for it (probably the side effects and complexity it could involve).

@ia3andy
Copy link
Contributor

ia3andy commented Feb 19, 2024

@gmuellerinform if you can, my recommendation would be to put your REST api on a specific sub-path (i.e /api), this way you can use the workaround suggested in the Quinoa doc:
https://docs.quarkiverse.io/quarkus-quinoa/dev/advanced-guides.html#spa-routing

@gmuellerinform
Copy link
Contributor

@ia3andy Thanks! This is what I am doing atm. But it is ughly... And I think a simple build time config could help us all:

initClassFactory, launchModeBuildItem.getLaunchMode(), servletPresent || !resumeOn404Items.isEmpty());

@geoand
Copy link
Contributor

geoand commented Feb 19, 2024

Sure, we can do that

@geoand geoand self-assigned this Feb 19, 2024
@geoand geoand removed the triage/needs-feedback We are waiting for feedback. label Feb 19, 2024
geoand added a commit to geoand/quarkus that referenced this issue Feb 19, 2024
This is a useful (albeit advanced) option for users
that want to use custom routes to handle
cases where RESTEasy Reactive does not match the
URL path.

Closes: quarkusio#38840
@geoand
Copy link
Contributor

geoand commented Feb 19, 2024

Here you go :)

@gmuellerinform
Copy link
Contributor

perfect, thanks a alot!!

@geoand
Copy link
Contributor

geoand commented Feb 19, 2024

🙏

geoand added a commit that referenced this issue Feb 19, 2024
Add configuration option for resuming on 404
@quarkus-bot quarkus-bot bot added this to the 3.9 - main milestone Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/vertx kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants