add req.toJSON() / res.toJSON() #4163

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
7 participants

tj commented Oct 18, 2012

otherwise JSON.stringify() throws, useful for logging etc

I don't really have any objections. We do it for Buffer as well.

👍

@TooTallNate TooTallNate commented on the diff Oct 19, 2012

test/simple/test-http-request-tojson.js
+var http = require('http');
+
+var server = http.createServer(function(req, res){
+ var obj = JSON.parse(JSON.stringify(req));
+
+ assert.deepEqual(obj, {
+ method: 'GET',
+ url: '/something',
+ headers: { host: 'localhost:' + common.PORT, connection: 'keep-alive' },
+ httpVersion: '1.1'
+ });
+
+ process.exit(0);
+});
+
+server.listen(common.PORT);
@TooTallNate

TooTallNate Oct 19, 2012

Is .listen() synchronous now? Or should the http.get() call be moved into a listen callback function?

@tj

tj Oct 19, 2012

forgot :D i'll fix this soonish

Can one of the admins verify this patch?

So this would need to be revamped to accommodate the split http.js mechanism, what I'd rather see instead of toJSON is a custom inspect method

Owner

jasnell commented May 19, 2015

... I assume this is like never going to be updated? Ok to close?

It should be okay to close, but I might open it as a "good first feature" issue on the converged node tracker and link back to this PR. Sorry @tj!

Owner

jasnell commented May 19, 2015

@chrisdickinson... Ok then, closing. Can reopen if necessary

jasnell closed this May 19, 2015

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