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

[Question or Feature Request] 404 error handling or sendFile on non existed route #298

Closed
pumano opened this issue Dec 10, 2017 · 18 comments

Comments

@pumano
Copy link
Contributor

pumano commented Dec 10, 2017

I'm submitting a...


[ ] Regression 
[ ] Bug report
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

How to handle 404 when route not existed or how to sendFile on non existed routes?

Expected behavior

I want to handle 404 not found properly. In pure express application I just add (after all routes):

  server.get('*', function (req, res) {
    res.send('not found');
  });

and it's works for all routes except previously defined by other routes. When I request non defined route I got 'not found'.

Minimal reproduction of the problem with instructions

I found a solution how to implement what I need, but it's not a 'Nest' way:

import * as http from 'http';
import * as express from 'express';

import { NestFactory } from '@nestjs/core';
import { ApplicationModule } from './modules/app.module';

const server = express();

async function bootstrap() {

  const app = await NestFactory.create(ApplicationModule, server);
  await app.init();

  http.createServer(server).listen(3000);

  server.get('*', function (req, res) {
    res.send('Not Found');
  });
}
bootstrap();

What is the motivation / use case for changing the behavior?

I want to implement that in Nest Way. I want to possibility to create some controller with '*' which can be loaded as latest route controller for catching all non existed requests for handling 404 or for sendFile aka static for frontend:

  // catch all unused routes to serve frontend
  server.get('*', function (req, res) {
    res.sendFile(path.join(__dirname, '../dist/index.html'));
  });

Environment


Nest version: 4.4.2

 
For Tooling issues:
- Node version: 8.9.0
- Platform:  Mac

Others:

@wbhob
Copy link
Contributor

wbhob commented Dec 10, 2017

You can use an interceptor to check if a response was sent back, and if it wasn't, send the error.

@pumano
Copy link
Contributor Author

pumano commented Dec 10, 2017

@wbhob Main reason is to sendFile for all not existed routes.

@wbhob
Copy link
Contributor

wbhob commented Dec 10, 2017

There isn't a good 'nest way' to do this because it maps all the URLs before the requests are instantiated, so there isn't an apparent hierarchy. You could try to maybe make a controller with the route * and make it the VERY last controller in your array.

@pumano
Copy link
Contributor Author

pumano commented Dec 10, 2017

How I can do it as very last controller?

@wbhob
Copy link
Contributor

wbhob commented Dec 10, 2017

Just make it the last one in your controllers array

@pumano
Copy link
Contributor Author

pumano commented Dec 10, 2017

@wbhob hope, I understand you, but I use modules. Each Module contain routes for example: /cars routes in CarsModule and /drivers in DriversModule.

I test your solution, and it works, but because I use modules, I need dedicated module and controller for it for using it as last module in app.module modules array. Thanks. Will be good if someone add that solution to documentation because too many people want to use Nest for API and also serve frontend on all unused routes (for example angular).

@kamilmysliwiec
Copy link
Member

Hi @pumano,
In the nearest update, Nest will throw NotFoundException when the non-existed route would be called. You'll be able to catch this exception in the exception filter, thus you can sendFile to the user then. Does it cover your requirements :)?

@wbhob
Copy link
Contributor

wbhob commented Dec 10, 2017

This is not the place for this type of question @ToporDon. please open a new issue, or better yet, as on Gitter.

@pumano
Copy link
Contributor Author

pumano commented Dec 11, 2017

@kamilmysliwiec good idea! Thank you!

Also will be good if you add docs (or I can help with it via pull request) about that NotFoundException and examples about how to catch NotFoundException and how to serve frontend via sendFile on NotFoundException. Hope it will be useful for Nest users.

@kamilmysliwiec
Copy link
Member

Hi @pumano,
Let's update your package into 4.5.0. I'll update the docs as soon as possible 🙂

@pumano
Copy link
Contributor Author

pumano commented Dec 18, 2017

If someone interested about how to do it.

not-found-exception.filter.ts:

import * as path from 'path';
import { ExceptionFilter, Catch, NotFoundException } from '@nestjs/common';
import { HttpException } from '@nestjs/core';

@Catch(NotFoundException)
export class NotFoundExceptionFilter implements ExceptionFilter {
  catch(exception: NotFoundException, response) {
    response.sendFile(path.join(__dirname, '../../../../dist/index.html'));
  }
}

server.ts

async function bootstrap() {
  const app = await NestFactory.create(ApplicationModule);
  app.useGlobalFilters(new NotFoundExceptionFilter());
  await app.listen(3000);
}
bootstrap();

@kamilmysliwiec
Copy link
Member

Thanks for sharing @pumano! 🙂

@silvelo
Copy link

silvelo commented Dec 21, 2017

Hi, @pumano, do you use app.setGlobalPrefix?

When I use it, this solution doesnt work, but without global prefix its works

@pumano
Copy link
Contributor Author

pumano commented Dec 21, 2017

@silvelo, no, I don't use app.setGlobalPrefix. maybe it's bug?

@silvelo
Copy link

silvelo commented Dec 21, 2017

Yes, maybe global configs make that issue.

@kamilmysliwiec
Copy link
Member

Actually, it's not a bug. The setGlobalPrefix mounts the Nest application on the applied 'global path'. You have to look forward for this RouterModule #255

@silvelo
Copy link

silvelo commented Dec 21, 2017

So at the moment I have two options:

  • Use the old way.
  • Delete global prefix.

Because my NestApplication doesnt response to any url wihtout that prefix.

Thanks @kamilmysliwiec

@lock
Copy link

lock bot commented Sep 25, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants