Explain chained method in order to implement a verbose mode about env-var HOT 9 CLOSED

I like the idea for "debug" capability.

But I think having to add explain to every call is a bit verbose. On the other hand does allow for granularity, in case we report any secrets.

I'm still leaving towards the global logger approach though.

from env-var.

On balance I think the debug module is best here.

It's common practice in the node community and it keeps the API of this module small.

I do like the proposed format of messages from above comments:

Reading RETRIES from environment : not found, using default of 5 (number)
Reading VTE_VERSION from environment : 5202 (string)

I'd be happy to accept a PR if anyone's interested in it. I can get around to it in a week or so if not.

from env-var.

I like this idea! I had considered adding some logging with the debug module before, but decided against it since I didn't want to add a dependency - your solution here is a good way to optionally support it 👍

In terms of the implementation, how about making it work similar to required and convertFromBase64? Those functions set boolean flags (see here) and those are used in the function (here) when reading the variable - you could just store the log function instead of a boolean.

It could default to a noop let out = () => {} , and explain(out) would set it to a real logger to do the logging?

from env-var.

Or as you say, default it to console.debug. We might just want to prefix logs with env-var so they're easy to search.

from env-var.

Great! Thanks for the steer, that was a big help. Having dabbled a little more, I was wondering if a slightly different approach would be better.

The issue (having thought about it) with the chained 'explain' is that it would be easy to miss - what if, somewhere fifty lines down, one of my team forgot to put '.explain' on a .get call. There would always be that niggling doubt in the back of my mind: "Am I really seeing all the environment vars that are in use here?"

Instead, I thought maybe adding a param to the 'from' constructor, something like:

const from = (container, extraAccessors, explainer) => {

then handle it in variable.js:

module.exports = function getVariableAccessors (container, varName, extraAccessors, explainer) {
  let doExplain = (typeof explainer === 'function')

then using 'doExplain' to conditionally add in the message construction and finally output it to the explainer function

This would avoid the need to call '.explain', with the (slight) tradeoff that the require now would look like:

const env = require('env-var).from(process.env,{}, console.debug)

Resulting in output more like this:

o.get('RETRIES').default('5').asInt()
env-var|Reading RETRIES from environment : not found, using default of 5 (number)
o.get('VTE_VERSION').default('5150').asInt()
env-var|Reading VTE_VERSION from environment : 5202 (number)
o.get('VTE_VERSION').default('5').asString()
env-var|Reading VTE_VERSION from environment : 5202 (string)

from env-var.

And one final thought - what if I use a couple of environment vars to optionally control the behaviour - for example:
ENV_VAR_DEBUG=true
ENV_VAR_TRUNC=30

The first is intended to enable debug mode (defaulting to console.debug), so no need for the developers to invoke the complex constructor - but the ops team can still enable if they need to see what is going on, without having to fiddle the code

The second because it occurred to me that some of the retrieved values may be quite long and it may be preferable to trunc the values upon output (using an ellipses to indicate truncation)

from env-var.

I thought maybe adding a param to the 'from' constructor, something like

Yeah, I like this. It's like a global on/off switch. Initially I was opposed to the ENV_VAR_DEBUG, but I think this is a great use case - no need for the developers to invoke the complex constructor - but the ops team can still enable if they need to

I'd rather avoid ENV_VAR_TRUNC though. Developers can enforce that in the explainer they pass if necessary.

So end result is something like this perhaps since it's a non-breaking change?

const { from, get } = require('env-var')

// I think varname might be useful to have passed to the explainer?
const env = from(process.env, {}, (varname, str) => {
  if (get('ENV_VAR_DEBUG').asBoolean() === true) {
    console.log(str)
  }
})

Alternatively this might be cleaner, but is a major version bump:

const { from, get } = require('env-var')

const env = from(process.env, {
  extraAccessors: {},
  explainer: (varname, str) => {
    if (get('ENV_VAR_DEBUG').asBoolean() === true) {
      console.log(str)
    }
  }
})

from env-var.

@Brigadier1-9 I just realised my example uses ENV_VAR_DEBUG too, but I agree with your idea that we can implement that logic internally 😄

Do you want to work on a PR for this?

from env-var.

@Brigadier1-9 I got around to an implementation of this that I think satisfies your requirements. Could you take a look at the 6.2.0 branch? The TLDR is that a logger can now be passed to from() to enable logging support.

from env-var.