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

Does not work for any chunked response #1

Open
theKashey opened this issue Jan 30, 2019 · 6 comments
Open

Does not work for any chunked response #1

theKashey opened this issue Jan 30, 2019 · 6 comments

Comments

@theKashey
Copy link

Literally - it stores only last chunk of data (is intercept.write supposed to work that way?), not the whole.

Anything lager than a few kb - would be broken.

@tprobinson
Copy link
Owner

tprobinson commented Feb 6, 2019

I haven't had any problems so far, but if there's a potential bug I'd love to know about it! Is this something you've seen in testing, or in theory? I've been using this module to cache images, which are always at least 50kb and in some cases larger than 3mb and it's been working for me. Which part looks like it's wrong to you specifically?

@sayore
Copy link

sayore commented Dec 5, 2019

Can confirm this, some images do not render after implementing this.

@tprobinson
Copy link
Owner

Could I get some example code that causes this to fail? As I mentioned, I've been using this on some fairly large images and so far haven't had issues with chunking. Not to say this isn't a valid issue, but I just need some way to be able to trigger it so I can figure out what's going on.

@sayore
Copy link

sayore commented Dec 6, 2019

It's not even a large image it seems to happen to a 91kb png image. Only a part of it gets rendered in the browser.

logo2.zip

Try to cache this image and use a simple img-tag(no css)
I don't know if this behaviour is only happening in my environment, i cant get the code as i didnt commit it when i tried to implement it into my project.

@theKashey
Copy link
Author

The problem is not here, it's at mung - https://github.com/richardschneider/express-mung/blob/master/index.js#L181-L208

It's just reporting to you everytime someone is calling .write, and that could be done multiple times in one request.
I've tried to undertand the "right" way to use mung for a multy-chunked case, and I am not sure it's usable - there is no "end" event, on which you might cache something... unless you override .end in the same way it overrides .write

@tprobinson
Copy link
Owner

tprobinson commented Dec 10, 2019

Ahh okay, so this is because of the .write function. After looking through my code, it makes sense that I wouldn't have run into this, because I haven't used that function.

Initially, I had problems with express-mung as well, so I made a forked version of it that this package uses: https://github.com/tprobinson/express-mung#sendFork

I don't recall if I altered how .write works in my fork, I'll take a look at that soon. (or if someone else does and sees an issue, please feel free to submit an issue / PR)

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

No branches or pull requests

3 participants