flush is deprecated in OutgoingMessage [iojs]

[jarrodconnolly]

Not sure if this should go here or in marko, trying here first.

The way this module is used by marko Template.render() ends up calling BufferedWriter.flush which is wrapping _http_outgoing.js OutgoingMessage. iojs at least (not sure if nodejs yet) has deprecated the flush message in favour of the flushHeaders method.

This is causing a console log message on every call to render() with the following message:

(node) OutgoingMessage.flush is deprecated. Use flushHeaders instead.

iojs: v2.5.0
marko: 2.7.9

[patrick-steele-idem]

Sharing the issue here is fine...

That doesn't make any sense since flush() and flushHeaders() would serve very different purposes and I don't even see flushHeaders() documented for OutgoingMessage/ServerResponse. We use flush() along with async fragments to force a gzip frame to be flushed when an async fragment is completed. This is mostly only applicable when gzip is enabled since most gzip middleware will buffer the entire response and without calling flush() the benefits of streaming would be negated. The flush() method is typically introduced by the compression middleware as shown in the following relevant code:
https://github.com/expressjs/compression/blob/c2af8bd8d5cec3577b40d61859ca3a0467052ded/index.js#L61-L65

In addition, AsyncWriter conditionally calls the flush() method only if it is implemented:

https://github.com/marko-js/async-writer/blob/5ef039139041fb40aadcdc0b7a10eb7d3d3a48b5/lib/AsyncWriter.js#L67-L69

I'm not sure what to suggest here, but I think we will need to do more investigation to see if we need to maybe file a bug against iojs to clear up the confusion.

Thank you for reporting the problem!

[jarrodconnolly]

I totally agree that flushHeaders would serve a different purpose, I will try to describe what I am seeing so far.

You can see the methods in OutgoingMessage here.

flushHeaders()
https://github.com/nodejs/io.js/blob/master/lib/_http_outgoing.js#L641

flush()
https://github.com/nodejs/io.js/blob/master/lib/_http_outgoing.js#L650

flush() still exists which is why it gets called of course but it logs the deprecated message every time now.

This behaviour seems very different that what nodejs is doing as can be seen here, both methods exist and are very different.
https://github.com/joyent/node/blob/master/lib/_http_outgoing.js#L597

Discussion about the deprecation change is here:
nodejs/node#1156

[patrick-steele-idem]

Thanks for sharing all of the details.

In your use case are you opening an HTTP request to another service and rendering HTML to the request body? I want to make sure I understand the problem completely to reproduce the issue.

For Node.js I see OutgoingMessage.prototype._flush() (with an underscore) and OutgoingMessage.prototype.flushHeaders() but I do not see OutgoingMessage.prototype.flush(). For io.js I see OutgoingMessage.prototype.flush() as deprecated with the original purpose to stupidly be "flush headers". See:
nodejs/node@b2e00e3

It looks like Node.js used to have flush() for OutgoingMessage, but they fixed it by completely removing flush() and replacing it with flushHeaders(). Here's a quote from @tjfontaine:

In order to preserve the potential for a flush method being added to the
streams API, rename flush to flushHeaders which is much more clear about
the behavior of this method.

Source: nodejs/node-v0.x-archive#9048

It looks like the io.js folks took the potentially meaningful flush() method and deprecated it because someone chose to give it a bad name a long time ago. The Node.js folks chose to also do the rename, but to not deprecate the old method so that it could be used in the future (which was probably the right thing to do despite breaking backwards compatibility).

Uggh...

Ideally, flush() should have always been an optional method to force any buffered data through the pipeline (not to "flush headers").

This small difference between io.js and Node.js is causing a problem for us.

You could do the following to make io.js more like Node.js, but it is a hack:

delete require('http').OutgoingMessage.prototype.flush;

Thoughts?

[jarrodconnolly]

My use case is very similar to this sample:
https://github.com/marko-js-samples/marko-express/blob/master/server.js

I am rendering a marko template directly to the express response object.

Here is a bit of the stack showing how I end up with this message from the marko side.

_http_outgoing.js:651
deprecated(), util.js:52
BufferedWriter.flush(), AsyncWriter.js:68
BufferedWriter.end(), AsyncWriter.js:74
AsyncWriter._finish(), AsyncWriter.js:485
AsyncWriter.handleEnd(), AsyncWriter.js:478
AsyncWriter.end(), AsyncWriter.js:441
Template.render(), marko-runtime.js:203

[Bluejay47]

Was there ever a resolution to this? I am seeing the same behavior.

[patrick-steele-idem]

Hey @Bluejay47 and @jarrodconnolly, I went ahead and filed an issue against the Node.js project: nodejs/node#2920

I see a few options:

• Option 1) Introduce a "no flush" option for marko
• Option 2) Ask application code to monkey patch the request object using delete req.flush
• Option 3) Monkey patch the prototype for OutgoingMessage: delete require('http').OutgoingMessage.prototype.flush;

Any other ideas? Thoughts?

[patrick-steele-idem]

I just verified that the problematic require('http').OutgoingMessage.prototype.flush method only ever existed it io.js (it was never in Node.js). For that reason, I recommend going with option 3:

delete require('http').OutgoingMessage.prototype.flush;

[Bluejay47]

I started receiving this message when I migrated my project to node 4. Is this related to the OP's error?

[patrick-steele-idem]

Yes, @Bluejay47. That console warning is the same issue. The following fix might feel wrong but it works

// Put the following line in your main script:
delete require('http').OutgoingMessage.prototype.flush;

io.js introduced the issue and it never existed in Node.js until the two merged with Node.js 4.0.0. If you look at the history you will see that someone used the bad judgement of introducing a flush() method that was a misnomer and then they quickly realized that was a mistake and renamed the method to flushHeaders() and deprecated the old flush() method (that marko calls if it exists).

[Bluejay47]

Thanks for the help.

[jarrodconnolly]

Thank you for following up and fixing this long standing and bizarre issue.