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

bug: "not found" handler intercepts already matched routes #36

Closed
7flash opened this issue Jun 4, 2024 · 7 comments
Closed

bug: "not found" handler intercepts already matched routes #36

7flash opened this issue Jun 4, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@7flash
Copy link

7flash commented Jun 4, 2024

I have a handler defined for my index page route as below:

app.get('/', function(req, res) {
  res.html(`<h1>Index Page</h1>`);
});

Then I want to add handler to show "not found" page for all other routes

app.get('*', function(req, res) {
  const accept = req.headers.get('Accept');
  if (accept.match(/text\/html/)) {
    res.html(render(`<h1>Page not found</h1>`));
  } else {
    res.fetch(req);
  }
});

Unfortunately when added this wildcard handler following after index route, then index route is not working anymore as expected and instead it shows "not found" message for defined index route as well.

@jcubic
Copy link
Owner

jcubic commented Jun 4, 2024

I need to think what to do. Wildcard grab everything, I'm not sure if this is a bug. I need to see if I can make it work as expected.

@7flash
Copy link
Author

7flash commented Jun 5, 2024

What can be a workaround to implement "not found" page in meantime?

@jcubic
Copy link
Owner

jcubic commented Jun 5, 2024

You will need to do your own routing with a single wildcard, but this will be almost the same as not using a library at all.

The routing library is not exposed, but it's based on route.js.

@jcubic
Copy link
Owner

jcubic commented Jun 5, 2024

It looks like this is how you add 404 page to express.js, so this is something that should be implemented.

@jcubic jcubic added the feature New feature or request label Jun 5, 2024
@jcubic
Copy link
Owner

jcubic commented Jun 5, 2024

So the workaround now is to use wildcard as first route:

app.get('*', function(req, res) {
  const accept = req.headers.get('Accept');
  if (accept.match(/text\/html/)) {
    res.html(render(`<h1>Page not found</h1>`));
  } else {
    res.fetch(req);
  }
});

app.get('/', function(req, res) {
  res.html(`<h1>Index Page</h1>`);
});

@jcubic jcubic added bug Something isn't working and removed feature New feature or request labels Jun 5, 2024
@jcubic
Copy link
Owner

jcubic commented Jun 5, 2024

It should work now as expected, the way route works, is search from the end and first found was used. Now if there are more than 1 routes it picks first non wildcard.

@jcubic
Copy link
Owner

jcubic commented Jun 6, 2024

I'm releasing next version to NPM.

@jcubic jcubic closed this as completed Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants