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

There is not good way to catch "File not found" errors on static assets #139

Open
tristan957 opened this issue Mar 21, 2020 · 19 comments
Open

Comments

@tristan957
Copy link

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

So if you setup the ServeStaticModule and provide serveStaticOptions: { fallthrough: false }, the module reports a 500 (Internal Server) error.

I don't know why this is not coming clearly to me, but I want to change the 500 error to a 404, because the file was not found.

I tried to use a global Internal Server filter and match the route in the case of 500 errors, but that wasn't possible because for some reason the filter doesn't get hit. I must be missing something higher level about NestJS.

Expected behavior

I expect there to be an ergonomic way to handle errors when fallthrough is set to false.

Minimal reproduction of the problem with instructions

Use the example in the nest repo: https://github.com/nestjs/nest/blob/master/sample/24-serve-static/src/app.module.ts#L8

and configure your ServeStaticModule to also serve your home directory (just an example) include serveStaticOptions: { fallthrough: false }. Then after you have set up the mount point, start the server and enter a URL under the mount point that would not exist in your home directory. Your browser should report a 500 error.

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

It currently seems either impossible or not easily discoverable on how a developer can catch the serve-static 500 error and change it to a 404 for when a file is not found.

Environment


Nest version: 7.0.2



Honestly maybe this is an issue for the serve-static repo. I am unsure.
@kamilmysliwiec
Copy link
Member

It currently seems either impossible or not easily discoverable on how a developer can catch the serve-static 500 error and change it to a 404 for when a file is not found.

Have you tried creating a global exception filter?

@tristan957
Copy link
Author

Yes. Sorry if that was not clear from the original issue. My exception filter aimed at catching these 500 errors in order to convert them to 404s.

The global exception filter does not get hit. I know this because my log statements did not come across the console. I can confirm that an instance of the exception filter was created, just not used. To create the global exception filter, I added a provider to my module using the documentation specified at docs.nestjs.com.

@kamilmysliwiec
Copy link
Member

Please, provide a minimal repository which reproduces your issue.

@danielkbx
Copy link

Just ran into the same issue. Even if an exception filter would be working, I think, a 404 would be the correct HTTP response code.

@tristan957
Copy link
Author

I'll see about getting a reproducible repo tonight.

@cyrilselasi
Copy link

cyrilselasi commented Apr 11, 2020

@kamilmysliwiec Run into the same issue. Was able to catch the error and handle it appropriately here:

@Catch()
export class ServeStaticExceptionFilter implements ExceptionFilter {

  catch( exception: any, host: ArgumentsHost ): any {

    const ctx = host.switchToHttp();
    const response = ctx.getResponse<Response>();
    const status = exception instanceof HttpException ? exception.getStatus() : HttpStatus.INTERNAL_SERVER_ERROR;

    if ( exception.code === "ENOENT" ) ) {
      response.sendStatus( HttpStatus.NOT_FOUND );

    }
    response.sendStatus( status );

    }
}

Main gotcha for me was the Catch decorator, since the error thrown is not an HttpException, be sure to leave catch everything.

@tristan957
Copy link
Author

@cyrilselasi have you found a good way to use the filter only on the route where you are serving static content?

@cyrilselasi
Copy link

Not yet, I have it set as a global filter at the moment. That's working fine for my case

@micaelmbagira
Copy link

@kamilmysliwiec @tristan957 here is a repo that reproduces the issue https://github.com/micaelmbagira/nestjs-service-static-139.

@cedx
Copy link

cedx commented Apr 17, 2020

Same issue here. Returning a 404 status code instead of catching the error 500 with a global exception filter is definitely the best option.

@kamilmysliwiec
Copy link
Member

PRs are more than welcome (if anyone is interested) :)

@tristan957
Copy link
Author

@kamilmysliwiec I think maybe all I will try later today. Does not seem too hard to add at least (if I understand the code correctly) for the Express side of things which I use.

@raphaelsoul
Copy link

Is it possible to disable index fallback to avoid this exception?

ServeStaticModule.forRoot({
      rootPath: path.join(__dirname, '../../../public'),
      serveStaticOptions: {
        index: false,
      },
    }),
[2021-03-25T10:35:11.080] [INFO] NestApplication - Nest application successfully started
[2021-03-25T10:35:13.787] [ERROR] ExceptionsHandler - ENOENT: no such file or directory, stat '/Users/raphaelsoul/projects/zuma/nodejs/mock-server/public/index.html' Error: ENOENT: no such file or directory, stat '/Users/raphaelsoul/projects/zuma/nodejs/mock-server/public/index.html'
[2021-03-25T10:35:13.788] [ERROR] ExceptionsHandler - ENOENT: no such file or directory, stat '/Users/raphaelsoul/projects/zuma/nodejs/mock-server/public/index.html' Error: ENOENT: no such file or directory, stat '/Users/raphaelsoul/projects/zuma/nodejs/mock-server/public/index.html'

@gdiasbruno
Copy link

index fallback

I have same issue

@micalevisk
Copy link
Member

Is it possible to disable index fallback to avoid this exception?

@raphaelsoul I don't think so due to the following:

const indexFilePath = this.getIndexFilePath(clientPath);

public getIndexFilePath(clientPath: string): string {
return join(clientPath, 'index.html');
}

@raphaelsoul
Copy link

raphaelsoul commented Feb 17, 2022

if (options.serveRoot) {
app.use(
options.serveRoot,
express.static(clientPath, options.serveStaticOptions)
);
const renderPath =
typeof options.serveRoot === 'string'
? options.serveRoot + validatePath(options.renderPath as string)
: options.serveRoot;
app.get(renderPath, renderFn);
} else {
app.use(express.static(clientPath, options.serveStaticOptions));
app.get(options.renderPath, renderFn);
}
});

res.sendFile in renderFn causes no such file error.

I think it is the best practice. we dont need a global exeception filter

ServeStaticModule.forRoot({
      rootPath: resolve(__dirname, '../public'),
      serveRoot: '/',
}),

For SPAs it is never a problem cause you will always have an index.html(or using index specify a filename)

For serving static file in traditional frontend project, just give a non-empty serveRoot.

Therefore, it seems there is no need for a PR to fix this, we just need update nestjs docs to declare usage for serving SPAs and non-SPAs
@kamilmysliwiec

#207 #523

UPDATE:

in some occasions, the solution above(give non empty serveRoot) still not works. For Example, you want to serve skins/material-design/assets under /themes/material-design.😆

ServeStaticModule.forRoot({
      rootPath: resolve(__dirname, '../skins/material-design/assets'),
      serveRoot: '/themes/material-design',
})

code below does not throw exceptions to built-in ExceptionsHandler class

ServeStaticModule.forRoot({
      rootPath: resolve(__dirname, '../skins/material-design/assets'),
      serveRoot: '/themes/material-design/', // NOTE: it end with slash `/`
})

you can check the reason here(for express)
image

every request url start serveRoot will be handled by express static middleware in line38 in the screenshot instead of renderFn function

It is very tricky. Seems we need robust code in @nestjs/serve-static library

@basz
Copy link

basz commented Mar 8, 2023

A other use case I have have is serving a static SPA where we need to inject data into the index.html. It needs to be routed through a controller.

Currently i can have a controller that handles /index.html but /some-other-route would serve the index.html unparsed.

@Lokkve0126
Copy link

It is possible to use async await in nestjs fastify to solve this problem

app.get(renderPath, async (req, res) => {
                    await new Promise((resolve, reject) => {
                        const stream = fs.createReadStream(indexFilePath);
                        stream.on("data", (chunk) => {
                            res.type('text/html').send(chunk);
                            resolve();
                        });
                        stream.on("error", () => {
                            reject(new NotFoundException()))
                        });
                    });
                });

Ambr3ak added a commit to tsiguenz/ft_transcendence that referenced this issue May 2, 2023
* fix error with [this link](nestjs/serve-static#139)

* Sidebar work in progress

* Sidebar work finished

* Navbar issues modif

* Navbar done

* navbar change button sign in and add sign up

* responsive bug

* Sidebar and navbar with searchbar done

* Add assets directory

* Fix after review

---------

Co-authored-by: Siguenza92 <thibaut.siguenza@gmail.com>
@tuanpq1998
Copy link

I'm currently encountering a "404 File Not Found" error when using the HTTP custom exception filter. This poses an issue because we have static files. Ideally, we'd like to provide a more user-friendly experience than just a JSON response or header status code. Is there a way to implement a colorful custom 404 error page using static files in NestJS? I haven't been able to find a method within NestJS itself, so I resorted to serving static files through Nginx and configuring error handling there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests