Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fixed bug in query streaming result. #1242

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants

seykron commented Dec 10, 2012

Steps to reproduce:

  1. Create a collection Things.
  2. Perform a query to list Things.
  3. Redirect output to http.ServerResponse.

Expected result:
Query results are streamed to the response.

Actual results:
A type error is thrown by http.ServerResponse.

TypeError: first argument must be a string or Buffer
    at ServerResponse.OutgoingMessage.write (http.js:715:11)

Fix details:
It seems that ServerResponse is expecting either a Buffer or a String, and
QueryStream is sending an object instead. Changed QueryStream to transform
the model instance to String via JSON.stringify.

Code to reproduce:

  /*
     ... Create a collection and save some kitties ...
  */
  var server = require("http").createServer();
  server.on("request", function (req, res) {
    Kitten.find({}).stream().pipe(res);
  });
Matías MI Fixed bug in query streaming result.
Steps to reproduce:
1) Create a collection Things.
2) Perform a query to list Things.
3) Redirect output to http.ServerResponse.

Expected result:
Query results are streamed to the response.

Actual results:
A type error is thrown by http.ServerResponse.

```
TypeError: first argument must be a string or Buffer
    at ServerResponse.OutgoingMessage.write (http.js:715:11)
```

Fix details:
It seems that ServerResponse is expecting either a Buffer or a String, and
QueryStream is sending an object instead. Changed QueryStream to transform
the model instance to String via JSON.stringify.
5040b40

seykron commented Dec 10, 2012

I wrote the example using http.ServerResponse but it applies to any WritableStream, according to Node Stream documentation.

@aheckmann aheckmann closed this Dec 10, 2012

Collaborator

aheckmann commented Dec 10, 2012

this isn't a bug, it is by design. its is up to the user to decide when to stringify the object (they may want to inspect it using its helper methods etc first)

seykron commented Dec 11, 2012

Mongoose documentation about query stream tells:

// follows the nodejs stream api
Thing.find({ name: /^hello/ }).stream().pipe(res)

And that doesn't work at all. How could it be by design if the WritableStream expects either a Buffer or String and it's receiving an object?

@aheckmann aheckmann referenced this pull request Dec 11, 2012

Closed

fix querystream docs #1243

Collaborator

aheckmann commented Dec 11, 2012

Ah thanks for pointing out the docs issue. I just opened #1243 to address.

On Monday, December 10, 2012, seykron wrote:

Mongoose documentation about query
streamhttp://mongoosejs.com/docs/api.html#query_Query-stream
tells:

// follows the nodejs stream api
Thing.find({ name: /^hello/ }).stream().pipe(res)

And that doesn't work at all. How could be it by design if the

WritableStream expects either a Buffer or String and it's receiving and
object?


Reply to this email directly or view it on GitHubhttps://github.com/LearnBoost/mongoose/pull/1242#issuecomment-11226672.

Aaron
@aaronheckmann https://twitter.com/#!/aaronheckmann

seykron commented Dec 11, 2012

Great, thanks. Just a comment: Node's Stream documentation specifies the Stream#data event as follows:

The 'data' event emits either a Buffer (by default) or a string if setEncoding() was used.

It should be nice to specify in mongoose's documentation that this contract is broken in favour of flexibility.

Collaborator

aheckmann commented Dec 12, 2012

Events:

data: emits a single Mongoose document

http://mongoosejs.com/docs/api.html#querystream-js

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