Comments (14)
Hi! What version of Node.js are you using? It doesn't seem to exist in 0.10, at least.
from compression.
Hi!
I'm using 0.12 (https://nodejs.org/api/stream.html#stream_writable_write_chunk_encoding_callback)
But I guess this parameter was there all along. That's the node way :-) (https://nodejs.org/docs/v0.10.0/api/stream.html#stream_writable_write_chunk_encoding_callback_1)
from compression.
This module is only for HTTP streams, which in Node.js are not actually the normal streams at all. For example, this is the actual source for the write
method we are overwriting in 0.10: https://github.com/joyent/node/blob/v0.10.24/lib/http.js#L841 and as you can see, it takes no callback
argument.
from compression.
It looks like the callback
argument was added in Node.js 0.12 sometime: https://github.com/joyent/node/blob/v0.12.0/lib/_http_outgoing.js#L409
from compression.
Looks like the callback argument to .write
was added in Node.js 0.11.6: nodejs/node-v0.x-archive@da93d6a
from compression.
I'm using Node 0.10.40. I supply the callback on second argument in res.write() and It's work but when compression middleware it throws an error:
TypeError: Unknown encoding: function (err){
if(err){
return console.log(err);
}
res.flush();
stream.resume();
}
which is my callback function. I try to fix it by change second arguments to "utf-8" and move callback to third argument. The error was gone but callback never get call.
update: I use node 4.1.2. Callback not get call too.
from compression.
I just ran into this problem as well and it took me a long time to figure out what was going on as it unexpectedly breaks res.write
in all middleware after this middleware is loaded. This bug prevents this middleware from being node 11+ compliant. If you ever add the compression middleware, all future calls to res.write
no longer call the callback.
var express = require('express');
var compression = require('compression')
var app = express();
app.use(compression())
app.get('/', function (req, res) {
res.write('Hello World!', 'utf-8', function() {
console.log('will never be called since compression overrode res.write without the callback parameter');
});
res.end();
});
app.listen(3000);
This is worse if you put the res.end
inside of the res.write
callback as your server will hang indefinitely.
Is there any update as to when this will be fixed? It seems simple enough of a fix if you just used _write.apply(res, arguments)
in your res.write
override so that all arguments will be preserved to the previous write function.
from compression.
There is the PR #80 that is actively being worked on that would resolve this.
from compression.
There seems to be a lot going on in that PR discussion wise. From what I can gather you seem to want two different PRs made: one that fixes this bug specifically and the other that fixes the flush()
bug. If I create a PR to fix this bug, do you see any problem with changing the res.write
and res.end
calls to use apply
to pass through the arguments properly?
from compression.
Hi @straker, yes there is a lot of discussion. The reference PR is specific to add the callbacks; the second PR to fix the flush bug was never made.
If I create a PR to fix this bug, do you see any problem with changing the res.write and res.end calls to use apply to pass through the arguments properly?
So there is more to fixing it than that, and there has been a lot of discussion around that very point in the PR :) The main point is that you cannot just pass the callbacks through, because otherwise, it will make the callbacks even more unpredictable on Node.js 0.8 and 0.10, which changes this module from "if you use this, you cannot use callbacks" to "if you use this, you can use callbacks, except on Node.js 0.8 and 0.10, where normally you cannot use callbacks, but now you can, but not if the client sends headers to not compress the output".
from compression.
Ah, I thought that PR was to fix the flush bug.
I'm not sure if I understand what you mean. How does passing through the arguments from compressions res.write
to nodes res.write
now allow callbacks in Node 0.8 and 0.10 if the res.write
node function doesn't accept a 3rd parameter in those version? All that would mean is that it receives an additional parameter that it ignores.
I thought that's what the two different PRs were suppose to separate because the PR you referenced looks(ed) at the argument list to compression's res.write
and then manually calls the callback if it exists. Changing compressions override of res.write
to call _write.apply(res, arguments)
instead of _write.call(this, chunk, encoding)
at the end still does not allow Node 0.8 and 0.10 to use callbacks (since the _write
does not accept them), but does allow Node 0.11+ to use them as per the spec. No functionality would be changed in those 2 versions of Node.
from compression.
Ah, I thought that PR was to fix the flush bug.
No, it doesn't, which was the point I was bringing up, that the PR is just trying to add callback support instead of actually fixing the flush bug.
How does passing through the arguments from compressions res.write to nodes res.write now allow callbacks in Node 0.8 and 0.10 if the res.write node function doesn't accept a 3rd parameter in those version?
Because the zlib stream in 0.8 and 0.10 does support the callbacks. You're welcome to also make a pull request if you have a different approach, but every time you keep talking about calling _write
, that would only apply to non-compressed responses, as compressed responses write to the zlib stream.
from compression.
I see, I misunderstood how that all worked together. I can see how that complicates maters significantly. I'll investigate further to see if I can come up with something else that will work taking into account zlib. Thanks for the responses.
from compression.
No problem, @straker! I would love to get this fixed, where it doesn't introduce even more head-aches for debugging :S I just have had so many more higher-priority issues to work around, and besides from this initial issue, I never even heard from anyone besides two just suddenly that it was even a big issue (probably because, when surveying npm
yesterday, almost every single module that overwrites res.write
/res.end
does not even pass through the callbacks, either, so it seems to be a fairly low-used API (at least among the users of those modules).
The Express.js TC has spoken about Node.js support recently, and we are wiling to start bumping majors to drop Node.js 0.8 support, but with Node.js 0.10 still supported by the Node.js foundation (which we are a part of) and still demand for Node.js 0.10 support among corporate, dropping 0.10 is still a bit iffy. I bring that up because dropping those versions would make this a non-issue...
from compression.
Related Issues (20)
- Setting Vary header although caching is disabled HOT 1
- "drain" event listener leak when using res.once("drain"); can't use res.removeListener("drain") HOT 3
- compresssion doesn't work ,the vue.txt is 2m HOT 2
- Content-Type: application/json; charset=utf-8 No effect HOT 2
- Question: Why this middleware HOT 2
- Corrupted compressed .js-files for Mac OS / Safari -clients HOT 11
- Is compression working when node server is running on a container? HOT 2
- middleware fails when the request has more than 1 values for accept-encoding header HOT 2
- Is compression result cached? HOT 1
- change Transfer-encoding HOT 1
- Why does the data size increase after compression HOT 1
- Force size to be a minimum... HOT 2
- Chunked encoding is broken after using this middleware HOT 1
- Using a current debug version HOT 1
- Deflate backwards HOT 7
- Compression instrumentation (before/after compression hooks) HOT 2
- Angular Not Compressing? HOT 2
- compression not working json payload HOT 6
- Crash when compressing characters like ū HOT 1
- Express returns a non-compliant HTTP/206 response when gzip is enabled HOT 9
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from compression.