lukeed / sirv

An optimized middleware & CLI application for serving static files~!
MIT License
1.07k stars 58 forks source link

Only half the image is served #55

Closed eurigilberto closed 4 years ago

eurigilberto commented 4 years ago

I am trying to use sirv but I run into a problem where only half of some of my images is being served. I haven't change any of the options other than the "dev" option which value is false. I would like to know if there is a way for me to find out if the problem is sirv or something else.

lukeed commented 4 years ago

Hi there,

Are you able to provide a reproduction of some kind?

eurigilberto commented 4 years ago

Hi, Thanks for the reply!

Right now I am not going to be able to show you a reproduction of the error, because it has not been consistent. I am developing a portfolio for my art using sapper (euriherasme.com). The code snippet below shows the server code where "sirv" is being loaded and my images are located on a folder called images inside the root folder of the project.

import sirv from 'sirv';
import polka from 'polka';
import compression from 'compression';
import * as sapper from '@sapper/server';
import bodyParser from 'body-parser';
import { shouldVerifyToken, verifyToken } from './middleware/tokenVerification.js';
import { shouldVerifyAdmin, verifyAdmin } from './middleware/adminVerification.js';

const { PORT, NODE_ENV } = process.env;
const dev = NODE_ENV === 'development';

const jsonParser = bodyParser.json();

const shouldParseRequest = (req) => {
    const bodyParserExcludedPaths = ['/api/uploadImage'];
    if (bodyParserExcludedPaths.find(val => val == req.path)) {
        return false;
    }
    return true;
}

polka()
    .use((req, res, next) => shouldVerifyToken(req) ? verifyToken(req, res, next) : next())
    .use((req, res, next) => shouldVerifyAdmin(req) ? verifyAdmin(req, res, next) : next())
    .use((req, res, next) => shouldParseRequest(req) ? jsonParser(req, res, next) : next())
    .use(
        sirv('./images', { dev }),
        sirv('static', { dev }),
        sapper.middleware()
    )
    .listen(PORT, err => {
        if (err) console.log('error', err);
    });

Sorry for not providing more information before, I though this was a known problem. I am not entirely sure what is causing the problem, I though it was that only half the image was saved on the cache of the internet browser, but I cleared my cache and it did not work.

As stated before, the error seems to be random, but when it happens I can do a "get" request to the server for that image and you can see that the image was not fully loaded. I am using pm2 to run the webpage, when the problem happens I stop the project and reload it the problem disappears. I think the problem may be with the cache system on the server.

lukeed commented 4 years ago

If this server restarts at all, or some process of it does (eg, via PM2) then you should not turn off dev-mode.

This is meant to be run with the lifetime of your production server. There's nothing special about the sirv cache other than it saturates at start up.

If you are wanting to use PM2, let everything die and come back up again.

As it is, there's too many moving parts to be able to provide any other insights or direction.

kidandcat commented 4 years ago

It's happening for me too, also using pm2, after a while, images are corrupt or only half of images are being served. If the process is restarted through pm2, everything will work correctly for a while.

eurigilberto commented 4 years ago

@kidandcat I would not say that the images are corrupt because the files are still there without any changes, but maybe the cached images are getting corrupted. The recommendation by @lukeed of having the dev-mode turned on all the time seems to do the trick, or at least it has not happened to me again since I did that.

kidandcat commented 4 years ago

Yeah, the good things with sapper and polka is that they both are very modular, so I just moved to serve-static. Anyway the issue is still there.

I think it is a memory leak, I suppose assets are cached at the start of the server, but it takes a while to start erroring. Also if it can helps, for me sometimes I have images which are being loaded in ranges and only the first one loads (cutted images) and other times the image doesn't load at all, the thing is that the server response looks okay, but probably the byte data is corrupted somehow (the size in the headers is always correct).

eurigilberto commented 4 years ago

Yeah, the server response always looks ok, and the size is always correct too. My images always loaded, and when one of them was "corrupt", at least 10% - 20% of the image loaded and it is always the same amount, meaning if an image loaded only 15% it would do that every time, it would not get worse or better.

lukeed commented 4 years ago

I'm not entirely sure how PM2 is operating behind the scenes, but I'm wondering if the GC is somehow related.

Also, what version are you both using? If you're using next, does it also happen in the stable (0.4.x) release + vice versa?

How long does the PM2 process need to be running before you start experiencing issues?

Thanks

eurigilberto commented 4 years ago

I am assuming GC = Garbage Collector Would there be a way to stop the garbage collector meddling with the cache? I am using:

"polka": "next",
"sirv": "^0.4.0"

I haven't changed the version of sirv, so I am not sure if updating it would change anything. I am not entirely sure how long it takes for the error to happen. The images are also cached on the client's browser and I usually don't clean the cache, so I would not see the error until after someone told me there was a problem or I tried the web page on incognito mode.

Thank you for creating polka and sirv, they are awesome.

imtiazmangerah commented 4 years ago

I am experiencing this as well - exactly as described above (response has correct headers, size etc with a corrupt image (half or fully transperent). Its worth noting additionally that it is a PNG image.

I am using sapper with polka, compression (1.7.4) and sirv (0.4.2), running in prod mode under pm2. I am serving around 12.54 req/min so it is a fairly busy server. On noticing the error the server had been up for 19 hours since the last restart.

Could compression be contributing to this?

eurigilberto commented 4 years ago

I am not entirely sure if compression could be contributing to the problem but I tried removing it entirely and it did not work (as you can see in my second reply, I had already removed compression at that point). It happens with or without compression, but I did not test how compression might affect it (how the time until the error appears changes).

Most of my images are PNG too.

imtiazmangerah commented 4 years ago

I have run a load test on a production build of the server running locally through pm2 - in total executed 329k requests to the server in ~10 minutes with 0 failures. The error did not occur during or after this load test.

Note the available RAM on my dev machine is higher than that of the production server (AWS ec2 t2.small). Not sure how else to attempt to reproduce it other than an extended load test on the exact same ec2 instance to account for any time or available memory based effects.

lukeed commented 4 years ago

Ok, sorry but you guys have to start including code snippets. I poked at this for a while and was not able to reproduce it even once. Without something to look at, this will just be a crossfire of what-ifs and unanswered questions.

You also all have PM2 as your common thread, which I find odd. So if 1+ of you start posting pieces of how you're booting sirv – and especially how your PM2 app is recovering – I think we'll start moving towards an answer.

Thanks

eurigilberto commented 4 years ago

This is what I am doing with mine:

var sirv = require('sirv');
const { PORT, NODE_ENV } = process.env;
const dev = NODE_ENV === 'development';

polka()
    .use(
        compression({ threshold: 0 }),
        sirv('static', { dev }),
        sirv('./images', { dev }),
        middleware()
    )
    .listen(PORT, err => {
        if (err) console.log('error', err);
    });

When you said this:

how your PM2 app is recovering

you meant "how are we getting our webpages to work again", right? If that is the case then, I only have to do (portfolio is my process name):

pm2 stop portfolio
pm2 reload portfolio

As I said in a previous reply, I got the error to disappear by passing: {dev = true} instead of the real state which would have been false when the error appeared.

imtiazmangerah commented 4 years ago

@lukeed it is basically a project bootstrapped using svelte sapper as follows:

npx degit "sveltejs/sapper-template#rollup" my-app The way it uses sirv is as follows in server.js:

import sirv from 'sirv';
import polka from 'polka';
import compression from 'compression';
import * as sapper from '@sapper/server';

const { PORT, NODE_ENV } = process.env;
const dev = NODE_ENV === 'development';

polka() // You can also use Express
    .use(
        compression({ threshold: 0 }),
        sirv('static', { dev }),
        sapper.middleware()
    )
    .listen(PORT, err => {
        if (err) console.log('error', err);
    });

The only unique bit is compared to a simple use case is probabally sapper.middleware(), which looks as follows:

function middleware(opts

 = {}) {
    const { session, ignore } = opts;

    let emitted_basepath = false;

    return compose_handlers(ignore, [
        (req, res, next) => {
            if (req.baseUrl === undefined) {
                let { originalUrl } = req;
                if (req.url === '/' && originalUrl[originalUrl.length - 1] !== '/') {
                    originalUrl += '/';
                }

                req.baseUrl = originalUrl
                    ? originalUrl.slice(0, -req.url.length)
                    : '';
            }

            if (!emitted_basepath && process.send) {
                process.send({
                    __sapper__: true,
                    event: 'basepath',
                    basepath: req.baseUrl
                });

                emitted_basepath = true;
            }

            if (req.path === undefined) {
                req.path = req.url.replace(/\?.*/, '');
            }

            next();
        },

        fs.existsSync(path.join(build_dir, 'service-worker.js')) && serve({
            pathname: '/service-worker.js',
            cache_control: 'no-cache, no-store, must-revalidate'
        }),

        fs.existsSync(path.join(build_dir, 'service-worker.js.map')) && serve({
            pathname: '/service-worker.js.map',
            cache_control: 'no-cache, no-store, must-revalidate'
        }),

        serve({
            prefix: '/client/',
            cache_control: dev ? 'no-cache' : 'max-age=31536000, immutable'
        }),

        get_server_route_handler(manifest.server_routes),

        get_page_handler(manifest, session || noop)
    ].filter(Boolean));
}

As for PM2, as far as I can tell it it does not restart the process and I have not used the --max-memory-restart flag. If there is anything more specific I could provide, please let me know. Thanks!

edit:

to get it to recover, I follow a similar process to @Gilberetzu. I use pm2 restart my-app

imtiazmangerah commented 4 years ago

If this helps, I have compared the file I expect it to serve and the file I receive when the error occurs. In my case, and in this instance, the file I expect to serve has 8065 bytes while the file I receive has 6017 bytes. The browser receives the file fully and with response of HTTP 200, and doing a binary comparison of the two shows that it is identical apart from the last 2048 bytes being truncated.

of interest is the number 2048 above. Could this be a clue?

eurigilberto commented 4 years ago

I am using the same svelte template as @imtiazmangerah and I am not using --max-memory-restart flag either. If there is something else you need, let me know.

vsviridov commented 4 years ago

Happening to me, just running npm run dev for a simple app... The bundle.js file gets cut off, doesn't help if I remove the folder contents, only restarting sirv makes it go away.

lukeed commented 4 years ago

Ok, again, need full details otherwise this doesn't help diagnose anything -- sorry.

Still waiting on a reproduction in order to move forward.

vsviridov commented 4 years ago

sirv-cli@1.0.4 Running from a svelte config by invoking sirv public/ directly No additional flags Bundle.js - 29Kb (empty app, basically) Sometimes fails after first recompile Not really predictable no pm2

I basically followed npx degit sveltejs/template my-svelte-project process to get the app going but updated all packages to latest.

I noticed that the Content-Length is set to 29023 and curl'd size on disk is 29023,

image

but original file size on disk is 29196. image

Maybe it caches the file size but when recompiled the bundle grows. sirv could be ignoring size change and sends old size.

imtiazmangerah commented 4 years ago

@lukeed when running with the dev flag set to true, the error does not occur. When running with the flag set to false it could occur, but it does not appear time based (uptime at the time of reproduction of error varies significantly despite fairly consistent load).

NB to others experiencing this, switching the dev flag to true is not a fix. It has performance and security implications (opens up possible directory traversal attack).

lukeed commented 4 years ago

@vsviridov thank you for information, but I'm still unclear what the CLI flags are. If you are developing/rebuilding files without --dev flag then it'll serve the cached file.

@imtiazmangerah Directory traversal has been resolved since v0.4.6 for dev and prod versions. Performance implications are still valid, though, which cannot be avoided.

imtiazmangerah commented 4 years ago

Submitted a PR (#75) that might fix this

lukeed commented 4 years ago

I think @imtiazmangerah's work in #75 does address this. It certainly seems plausible and would make sense.

If anyone still runs into this with (or after) sirv@1.0.5 or sirv-cli@1.0.5 (just published), please let me know and we can reopen this ticket.

Thanks!

ezfe commented 3 years ago

Hi @lukeed,

I have sirv-cli@1.0.11 installed and I've started getting incomplete bundle.js as well as timeouts despite the entire file being loaded–all of this is with the on-disk version being complete.

In one instance, it trimmed off the last ~48 characters. In another, it served the entire file but then timed out. When running in curl, the following error showed for the second situation: curl: (18) transfer closed with 31 bytes remaining to read

--

I'm running sirv using the livereload default configuration from Svelte, and I notice it breaks whenever the live reload kicks off. Working on getting more info for you

lukeed commented 3 years ago

Hey, thanks! Maybe (one of) you could send me the failing image and/or bundle.js that's causing this to happen? I still don't run into this in my own work ever, so I'd need something that I can actually test with. I have a possible idea of what this might be, but would still need something to test against.

I can be found on discord if you don't want to upload files publicly :)

ezfe commented 3 years ago

You can use my project, https://github.com/ezfe/tellraw in the formatted-translations branch. If you run npm run dev, load the page up, make a change (in Tellraw.svelte is a solid location) that triggers rebuild, this issue occurs.

Here's a screen recording that shows this happening:

I pressed command+S to save after adding a change in VSCode, but did not press reload in Safari (relied on auto reloader)

https://user-images.githubusercontent.com/1449259/111847670-a909ac00-88df-11eb-92de-a0ddc2e5e97e.mov

resistcorp commented 3 years ago

Hi, just to say I'm hitting the same problem with a svelte + rollup app.

I suspect a header bug in sirv, but it could (maybe) be in rollup livereload I cloned @ezfe 's repo and confirmed that the problem is reproducible on windows, with the browser reporting a ERR_CONTENT_LENGTH_MISMATCH image

environment : windows browser : chrome sirv version : 1.0.12 reproduction (as seen in above video ) :

I'll be switching to another way of serving my bundle in dev mode but I'm interested in knowing more about this bug as sirv is the most painless way (usually)