An Empty Environment Variable Is Set. My Code Disagreed.

LOG_TIMESTAMP="" is not the same as not setting LOG_TIMESTAMP. One of those is a user who typed something and got it wrong. The other is a user who never typed anything. My config loader collapsed them into the same branch, and the bug it produced is one I’d never have found by reading the code, because the output was correct.

Here’s the guard:

if (env.LOG_TIMESTAMP) {
  config.timestamp = env.LOG_TIMESTAMP.toLowerCase() === 'true';
}

Empty string is falsy in JavaScript. So LOG_TIMESTAMP="" skips the branch, config.timestamp stays unset, and the logger falls through to its default. Which is the right value! An empty string isn’t a valid boolean, so ignoring it is exactly what should happen.

What doesn’t happen is the warning.

The spec says be noisy about it

The contract this library is judged against has a clause, §6.3, saying an environment variable set to a value the implementation doesn’t recognize must be ignored and diagnosed. Ignore the garbage, then tell the user you ignored it, because a config value that silently does nothing is how people lose an afternoon.

An empty string is unrecognized. So LOG_TIMESTAMP="" should ignore the value and print something like:

[logan-logger] LOG_TIMESTAMP="" is not a recognized boolean and was ignored.
Accepted: true, 1, yes, on / false, 0, no, off.

Instead it printed nothing, and did the right thing, and looked from the outside exactly like a user who hadn’t set the variable at all. Somebody with a broken .env line, a shell expansion that came back empty, a Kubernetes ConfigMap key with no value, all of them get silence and the default.

Now think about what kind of test catches that. Not an output comparison, because the output is identical either way. Not a value assertion, because the value is right. The only thing that distinguishes the bug from correct behavior is the presence of a warning nobody wrote a test for. You have to be asserting on diagnostics, and you have to be asserting on their absence too.

This bug can’t happen in Go or Rust

I mean that as a claim about the languages, not a compliment to myself, and it’s the part I keep chewing on.

In Go you cannot write if os.Getenv("LOG_TIMESTAMP"). It doesn’t compile. There’s no truthiness, so you’re forced into an explicit comparison, and at that moment you have to decide what you meant. If you want presence, the standard library hands you a two-value form that makes the distinction impossible to skip:

value, ok := os.LookupEnv("LOG_TIMESTAMP")

ok is presence. value is content. They’re separate variables because they’re separate questions.

Rust does the same thing through the type system. env::var gives you a Result<String, VarError>, and VarError::NotPresent is a distinct variant from Ok(""). You can’t accidentally treat an empty value as an absent one, because they aren’t the same shape and the compiler will make you handle both.

JavaScript gives you string | undefined and an idiom, if (value), that quietly folds undefined and "" together along with "0" and NaN. The idiom is so normal that it reads as correct in review. I wrote it, I reviewed it, and it survived three releases in a section of the code I’d already been editing.

The fix is one operator and a comment explaining why the obvious version is wrong:

// Presence, not truthiness. An empty string is set, matches no accepted value,
// and so takes the unrecognized path including its diagnostic (SPEC 6.3). A
// truthy guard skips it silently: the resulting value stays accidentally right
// while the required warning disappears.
if (env.LOG_TIMESTAMP !== undefined) {
  const timestamp = parseBooleanEnvironment('LOG_TIMESTAMP', env.LOG_TIMESTAMP);

  if (timestamp !== undefined) {
    config.timestamp = timestamp;
  }
}

!== undefined on all four variables. Empty takes the unrecognized path and warns. A variable that was never set stays silent, which is also required, because warning about variables the user never set would be its own kind of useless.

How you test for silence

The conformance runner found this, not code review. Which meant teaching it a vocabulary it didn’t have: input.env for ambient environment, input.files for config files on disk, and expect.diagnostics for what the implementation should have said.

The subtle part is the assertion that nothing was printed. That test is only meaningful if you control the environment completely, and a developer’s shell does not cooperate. If I’ve got LOG_LEVEL=debug exported in my terminal from an hour ago, a case asserting “no diagnostic for an unset variable” will fail on my machine and pass in CI, or the reverse, and either way the suite is now reporting on my shell rather than on the library.

So the runner clears all four variables for any case that carries an env block, then applies only what the case declares. It restores the environment afterward including on failure, and removes temp directories including on failure. It doesn’t use process.chdir(), since that’s process-global state and a parallel test runner will happily hand you somebody else’s working directory.

There’s one more guard I’d recommend to anybody building this kind of harness. A fixture format that silently ignores keys it doesn’t understand is a fixture format where a typo becomes a vacuous pass. Write expect.diagnostic instead of expect.diagnostics and your case asserts nothing while reporting green. So: an unknown input key throws, an unknown expect key throws, and a known expect key that the chosen assertion kind never actually produces fails as undischarged. The suite has to be unable to pass by accident before you can believe anything it tells you.

Conformance went from 62 to 119 cases with that work, covering the config and file sections for the first time. It’s at 129 today, and I ran it while writing this. All green, 323ms.

Four things worth keeping:

  1. if (env.FOO) is a bug in config code. Use !== undefined and decide what an empty value means on purpose.
  2. A correct value can hide a missing diagnostic. If your spec requires a warning, assert on the warning, not just the resulting config.
  3. Test the silence too, and clear the ambient state first, or you’re testing your shell.
  4. Make your fixture format reject unknown keys. A typo that produces a vacuous pass is worse than no test.

The boolean parser now accepts true, 1, yes, on and false, 0, no, off. The old one only matched the literal string true, so LOG_TIMESTAMP=1 used to mean false. Nobody hit that, because at the time nothing read the result anyway. That’s a different post.

Sources

  • Node.js: process.env — where an unset variable and an empty one both arrive as something falsy
  • MDN: Truthy — the coercion table, including the empty string
  • Go: os.LookupEnv — presence and value as separate return values
  • Rust: std::env::var and VarErrorNotPresent as a distinct variant from an empty Ok
  • PR #84 — the fix, the ambient-state vocabulary, and the anti-vacuous guards
  • treering — the spec and fixtures that caught it

I’d appreciate a follow. You can subscribe with your email below. The emails go out once a week, or you can find me on Mastodon at @[email protected].

Testing javascript Typescript Configuration