Custom logs with frame object as the 2nd param #55

[zatziky]

This is a proposal for #55

@JSteunou

[JimiC]

If you are not going to use frame inside the function, passing it as a parameter is redundant.

[zatziky]

true, I will remove the param

[JimiC]

Removing the param raises another question. The whole PR ends up moving let frame = new Frame(command, headers, body); from marshall to _tramsit with no obvious gain as frame will not be used as a second param to debug function. The PR looks kinda pointless. I wonder if there is something else I don't see.

[zatziky]

Yes, there is when overriding the client.debug then you have access to command, headers and body separately and can log what you need. We are for example truncation Authorization header:

client.debug = (text, frame) => {
    if (frame.headers && frame.headers.Authorization) {
      const newHeaders = evolve({
        Authorization: truncate
      }, frame.headers)
      log.debug(_frameToString(frame.command, newHeaders, frame.body))
    }
  }

It's really much easier to read the logs now as our JWT was ocupying 3 console lines. We may soon strip some content of the body or completely remove some headers.

[JimiC]

And why is the current signature not suiting you? I mean from what I know (text, frame) complies with (...args). Declaring

client.debug = (...args) => {
    if (args[1].headers && args[1].headers.Authorization) {
      const newHeaders = evolve({
        Authorization: truncate
      }, args[1].headers)
      log.debug(_frameToString(args[1].command, newHeaders, args[1].body))
    }
  }

and calling debug(text, frame) should work the same.

[zatziky]

I am using ES6 but it can be rewritten to vanilla very easily. Maybe the ramda library is confusing you?

[JimiC]

I'm just asking cause I realized that we are not providing typescript types for the debug function and this will definitely cause problems for those who want to override the function.

[zatziky]

I've never used Typescript but it's another obstacle. What do you say about using the other function debugWithFrame(frame, ...args). I definitely think it's necessary for us to have a finer way to pick what we would include in our logs than parsing a string.

[JimiC]

I'm not familiar with Ramda (man there are so many libraries out there), but I stick to Zakas quote that named arguments in ECMAScript are a convenience and not a necessity.

[zatziky]

Nevermind, why not. I'll wait with further changes until we agree on them. @JSteunou @JimiC

[zatziky]

@JimiC check it out again. I've removed the unused param frame

[JSteunou]

@zatziky please do not push dist files. Only src.

[JimiC]

According to Professional JavaScript for web developers by Nicholas C. Zakas

---

[zatziky]

@JimiC I know this but I didn't realize what you want me to see.

[JSteunou]

closed in favour of #58