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

update #3

Closed
wants to merge 2 commits into from
Closed

update #3

wants to merge 2 commits into from

Conversation

DomiR
Copy link

@DomiR DomiR commented Jan 24, 2017

No description provided.

@joepie91
Copy link
Owner

joepie91 commented Jan 24, 2017

Closing this because an up-to-date version of the repository exists elsewhere (see #2), and because the logging of the full task object is intentional for debugging purposes.

If you'd like to log in a different manner for your application (eg. only logging up to a certain depth), you can specify a custom event handler that logs in the desired manner.

@joepie91 joepie91 closed this Jan 24, 2017
@DomiR
Copy link
Author

DomiR commented Jan 24, 2017

The problem with logging is, that even though I don't ever want to log it, the object passed in a task will be serialized by util.inspect(..null..) no matter what, which is not performant at all.

E.g. when handling large objects like database pointers e.g. rethink. My process just ate 600mb of memory because of that.

@joepie91
Copy link
Owner

joepie91 commented Jan 24, 2017

Ah yes, I see the problem. While I don't think it's a good idea to pass stateful objects in the task data (what if the connection is lost before the task runs?), this is still not acceptable behaviour on promise-task-queues part.

Either way, limiting the util.inspect depth isn't really the right solution here. It seems that debug supports custom formatters, and a quick glance at the code shows that the formatter logic is only executed when a debug namespace is enabled - so the solution would probably be to move the custom util.inspect call into a custom formatter.

I've created an issue for this over on the new repository, and I'll fix it tomorrow. It's getting late here, and it's a bad idea to publish releases when tired :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants