Skip to content

get_env() crashes when find_dotenv()/dotenv_values() fails (FileNotFoundError/OSError) — unguarded dotenv fallback #5281

Description

@aivong-openhands

Summary

openhands/sdk/observability/utils.py::get_env() calls dotenv_values() with no path, which internally calls python-dotenv's find_dotenv(). find_dotenv() is not robust to abnormal execution contexts — when the process working directory has been removed, or the call-stack / frame walk is unusual, it raises an OSError (e.g. FileNotFoundError: [Errno 2] No such file or directory). Because get_env() does not guard the call, the exception propagates to the caller.

get_env() is used by the observability enablement check (should_enable_observability), so an unrelated dotenv failure can break code paths that merely check whether observability should be enabled. This is not a Laminar/tracing bug — the defect is in the shared get_env() helper, which should tolerate find_dotenv()/dotenv_values() failing and fall back to the environment only.

Current code

openhands-sdk/openhands/sdk/observability/utils.py:

def get_env(key: str) -> str | None:
    """Get an environment variable from the environment or the dotenv file."""
    return os.getenv(key) or dotenv_values().get(key)

dotenv_values() with no argument -> find_dotenv() -> os.getcwd() / frame walk -> raises OSError/FileNotFoundError in some contexts. There is no error handling.

Steps to reproduce

find_dotenv() calls os.getcwd(), which raises FileNotFoundError if the current working directory no longer exists:

import os, tempfile
from openhands.sdk.observability.utils import get_env

d = tempfile.mkdtemp()
os.chdir(d)
os.rmdir(d)          # CWD now deleted
get_env("SOME_KEY")  # raises FileNotFoundError: [Errno 2] No such file or directory

Example traceback (via the observability enablement check):

File "openhands/sdk/observability/laminar.py", should_enable_observability
File "openhands/sdk/observability/utils.py", line 10, in get_env
File "dotenv/main.py", dotenv_values
File "dotenv/main.py", find_dotenv
FileNotFoundError: [Errno 2] No such file or directory

Relationship to prior reports

  • find_dotenv breaks local conversation #1325 (find_dotenv breaks local conversation, CLOSED as COMPLETED) — the same get_env -> dotenv_values -> find_dotenv code path, reported from the CLI/local-conversation entry point. At that time the failure surfaced as AssertionError (older python-dotenv, which had assert frame.f_back is not None in find_dotenv). It was closed with "We no longer see this error" after the stale bot flagged it — i.e. the symptom disappeared (most likely because python-dotenv was upgraded from 1.1.x to 1.2.x, changing/removing that assertion), not because the underlying get_env() fragility was fixed.
  • Fix find_dotenv assertion error in local conversation #1326 (Fix find_dotenv assertion error in local conversation, CLOSED, never merged) — proposed wrapping get_env() to catch AssertionError and OSError and return None. That fix would have covered this FileNotFoundError (an OSError), but it was never merged, so get_env() remains unguarded on main.

Net effect: the fragility identified in #1325 was never actually fixed, and can still occur under a different exception type (OSError/FileNotFoundError).

Proposed fix

Harden get_env() so a failing dotenv lookup never propagates — check the environment first, and treat any dotenv_values()/find_dotenv() failure as "no dotenv value":

def get_env(key: str) -> str | None:
    """Get an environment variable from the environment or the dotenv file."""
    value = os.getenv(key)
    if value is not None:
        return value
    try:
        return dotenv_values().get(key)
    except (OSError, AssertionError):
        return None

(This is essentially what abandoned PR #1326 proposed; catching OSError covers FileNotFoundError.)

Acceptance criteria

  • get_env() returns the environment value when set, without invoking dotenv.
  • get_env() returns None (does not raise) when dotenv_values()/find_dotenv() raises OSError/FileNotFoundError/AssertionError.
  • Regression test covering a missing/deleted working directory or an otherwise failing find_dotenv().

This issue was filed with the help of an AI agent (OpenHands).


OpenHands AI triage

The following comments and acceptance criteria were added by the OpenHands AI agent.

Triage

Confirmed against main (v1.49.5): openhands-sdk/openhands/sdk/observability/utils.py::get_env() is still return os.getenv(key) or dotenv_values().get(key), so an unguarded dotenv_values() -> find_dotenv() call can raise. Reproduced locally with python-dotenv 1.2.2 (the version pinned in uv.lock): find_dotenv() raises AssertionError when the process working directory is deleted, and FileNotFoundError ([Errno 2]) on the os.getcwd() branch, matching the production traceback in this report. The defect is the shared helper, not Laminar - Laminar (should_enable_observability) is just the caller.

Bounded scope: harden get_env() so the environment is consulted first and a failing dotenv lookup degrades to "no dotenv value"; add a focused regression test alongside the existing tests/sdk/observability/test_laminar.py.

Explicit non-goals: no change to Laminar/observability enablement semantics or the _observability_enabled cache; no change to where dotenv files are searched; no new runtime dependency; no re-litigating the closure of #1325/#1326; no caching of dotenv results. Note that the environment now takes precedence whenever the variable is present, including an empty string (the current or fallback to .env for an empty-but-set variable is intentionally dropped).

Actual Behavior

When the process working directory has been removed (or the find_dotenv() frame walk is otherwise abnormal), get_env() lets the exception from dotenv_values()/find_dotenv() propagate. It surfaces through should_enable_observability() in the agent-server websocket path as FileNotFoundError: [Errno 2] No such file or directory (caught and logged as error_in_subscription; non-fatal to the pod, but noisy per websocket event push).

Steps to Reproduce

  1. Use python-dotenv 1.2.2 (as pinned in uv.lock).
  2. From Python, os.chdir(tmpdir) then delete tmpdir so the working directory no longer exists.
  3. Call openhands.sdk.observability.utils.get_env("LMNR_PROJECT_API_KEY") with the variable unset in the environment.
  4. Observe the exception propagate (AssertionError on the REPL/file-walk branch, FileNotFoundError on the os.getcwd() branch) instead of get_env() returning None.

Desired Behavior

get_env(key) returns the environment value when the variable is set, and otherwise returns None when the dotenv lookup fails for any reason, never propagating OSError/FileNotFoundError/AssertionError to callers such as should_enable_observability().

Acceptance Criteria

  • get_env(key) returns the process environment value whenever the variable is present (including an empty string) and does not consult dotenv_values() in that case.
  • When the variable is unset and dotenv_values()/find_dotenv() raises FileNotFoundError, OSError, or AssertionError, get_env(key) returns None instead of raising.
  • should_enable_observability() returns False (no exception) when called with a removed/absent working directory and no observability env var set.
  • Normal dotenv behavior is preserved: with a readable .env containing the key and the variable unset in the environment, get_env(key) still returns the .env value.
  • A regression test under tests/sdk/observability/ exercises get_env() against a failing/deleted working directory and asserts the non-raising None result; it passes via uv run pytest tests/sdk/observability.
  • No new runtime dependency is added; python-dotenv remains the existing dependency and dependency declarations are unchanged.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

priority:lowFor bugs, affects only non-mainstream cases, or is annoying but with a clear workaround.ready-for-devIssue meets development readiness criteria

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions