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

implement debugging to file and console. Fixes #72 #74

Conversation

@danielb2
Copy link

danielb2 commented Mar 3, 2015

No description provided.

@danielb2

This comment has been minimized.

Copy link
Author

danielb2 commented Mar 3, 2015

Failing tests are due to req.abort() not functioning

@geek geek added this to the 5.3.0 milestone Mar 4, 2015
@geek geek self-assigned this Mar 4, 2015
@geek geek added the feature label Mar 4, 2015
@geek

This comment has been minimized.

Copy link
Member

geek commented Mar 4, 2015

@danielb2 the new feature needs to be documented in the readme

@@ -87,6 +90,18 @@ exports.request = function (method, url, options, callback, _trace) {
req.abort();
}

if (logger.active) {

This comment has been minimized.

Copy link
@geek

geek Mar 4, 2015

Member

lets simplify this to logger.log(err, url, options, req, res); and make the logger wrap the res stream when it needs to output details from it. What I mean is, inside of log if we are going to need to read the res then do it then... but move that logic into logger.

@@ -72,6 +74,7 @@ exports.request = function (method, url, options, callback, _trace) {
}

var req = client.request(uri);
req.time = new Date().getTime();

This comment has been minimized.

Copy link
@geek

geek Mar 4, 2015

Member

we need to find a safer place to store this data. We can create a now var that contains this info and pass it into the log function as options.

@danielb2 danielb2 force-pushed the danielb2:issue.72-Add.logging.to.req/res.details.for.debug.mode branch 2 times, most recently from 5a0c202 to 45e4805 Mar 4, 2015
logger.log(logData);
}
else {
exports.read(res, null, function (err, payload) {

This comment has been minimized.

Copy link
@geek

geek Mar 4, 2015

Member

this is going to be an expensive operation... this should only happen when we have logging enabled. Please only do this part in the log function.

@danielb2 danielb2 force-pushed the danielb2:issue.72-Add.logging.to.req/res.details.for.debug.mode branch from c0c7981 to 1e89a2a Mar 5, 2015
This was referenced Mar 10, 2015
@geek geek closed this Mar 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.