Improve error handling and clarity in catch_dotenv hook and tests

2026-03-29 10:16:52 +00:00 · 2025-08-28 20:29:12 -06:00 · 2025-08-28 20:29:12 -06:00 · 989ac68f29
commit 989ac68f29
parent 33746f52ec
3 changed files with 138 additions and 23 deletions
--- a/pre_commit_hooks/catch_dotenv.py
+++ b/pre_commit_hooks/catch_dotenv.py
@ -19,22 +19,30 @@ _KEY_REGEX = re.compile(r"^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=")


 def _atomic_write(path: str, data: str) -> None:
-    """Atomic-ish text write: write to same-dir temp then os.replace."""
+    """Atomically (best-effort) write text.
+
+    Writes to a same-directory temporary file then replaces the target with
+    os.replace().  This is a slight divergence from most existing hooks which
+    write directly, but here we intentionally reduce the (small) risk of
+    partially-written files because the hook may be invoked rapidly / in
+    parallel (tests exercise concurrent normalization).  Keeping this helper
+    local avoids adding any dependency.
+    """
    fd, tmp_path = tempfile.mkstemp(dir=os.path.dirname(path) or ".")
    try:
        with os.fdopen(fd, "w", encoding="utf-8", newline="") as tmp_f:
            tmp_f.write(data)
        os.replace(tmp_path, path)
    finally:  # Clean up if replace failed
-        if os.path.exists(tmp_path):  # pragma: no cover (rare failure case)
+        if os.path.exists(tmp_path):  # (rare failure case)
            try:
                os.remove(tmp_path)
-            except OSError:  # pragma: no cover
+            except OSError:  
                pass


-def ensure_env_in_gitignore(env_file: str, gitignore_file: str, banner: str) -> bool:
-    """Normalize .gitignore tail (banner + env) collapsing duplicates. Returns True if modified."""
+def _read_gitignore(gitignore_file: str) -> tuple[str, list[str]]:
+    """Read and parse .gitignore file content."""
    try:
        if os.path.exists(gitignore_file):
            with open(gitignore_file, "r", encoding="utf-8") as f:
@ -45,14 +53,17 @@ def ensure_env_in_gitignore(env_file: str, gitignore_file: str, banner: str) ->
            lines = []
    except OSError as exc:
        print(f"ERROR: unable to read {gitignore_file}: {exc}", file=sys.stderr)
-        return False
-    original_content_str = original_text if lines else ""  # post-read snapshot
+        raise
+    return original_text if lines else "", lines

+
+def _normalize_gitignore_lines(lines: list[str], env_file: str, banner: str) -> list[str]:
+    """Normalize .gitignore lines by removing duplicates and adding canonical tail."""
    # Trim trailing blank lines
    while lines and not lines[-1].strip():
        lines.pop()

-    # Remove existing occurrences (exact match after strip)
+    # Remove existing occurrences
    filtered: list[str] = [ln for ln in lines if ln.strip() not in {env_file, banner}]

    if filtered and filtered[-1].strip():
@ -62,14 +73,35 @@ def ensure_env_in_gitignore(env_file: str, gitignore_file: str, banner: str) ->

    filtered.append(banner)
    filtered.append(env_file)
+    return filtered

-    new_content = "\n".join(filtered) + "\n"
-    if new_content == (original_content_str if original_content_str.endswith("\n") else original_content_str + ("" if not original_content_str else "\n")):
+
+def ensure_env_in_gitignore(env_file: str, gitignore_file: str, banner: str) -> bool:
+    """Ensure canonical banner + env tail in .gitignore.
+
+    Returns True only when the file content was changed. Returns False both
+    when unchanged and on IO errors (we intentionally conflate for the simple
+    hook contract; errors are still surfaced via stderr output).
+    """
+    try:
+        original_content_str, lines = _read_gitignore(gitignore_file)
+    except OSError:
        return False
+
+    filtered = _normalize_gitignore_lines(lines, env_file, banner)
+    new_content = "\n".join(filtered) + "\n"
+
+    # Normalize original content to a single trailing newline for comparison
+    normalized_original = original_content_str
+    if normalized_original and not normalized_original.endswith("\n"):
+        normalized_original += "\n"
+    if new_content == normalized_original:
+        return False
+
    try:
        _atomic_write(gitignore_file, new_content)
        return True
-    except OSError as exc:  # pragma: no cover
+    except OSError as exc: 
        print(f"ERROR: unable to write {gitignore_file}: {exc}", file=sys.stderr)
        return False

@ -107,7 +139,7 @@ def create_example_env(src_env: str, example_file: str) -> bool:
        _atomic_write(example_file, "\n".join(header + body) + "\n")
        return True
    except OSError as exc:  # pragma: no cover
-        print(f"ERROR: unable to write '{example_file}': {exc}")
+        print(f"ERROR: unable to write '{example_file}': {exc}", file=sys.stderr)
        return False


@ -116,26 +148,25 @@ def _has_env(filenames: Iterable[str], env_file: str) -> bool:
    return any(os.path.basename(name) == env_file for name in filenames)


-
-
 def _print_failure(env_file: str, gitignore_file: str, example_created: bool, gitignore_modified: bool) -> None:
-    parts: list[str] = [f"Blocked committing {env_file}."]
+    # Match typical hook output style: one short line per action.
+    print(f"Blocked committing {env_file}.")
    if gitignore_modified:
-        parts.append(f"Updated {gitignore_file}.")
+        print(f"Updated {gitignore_file}.")
    if example_created:
-        parts.append("Generated .env.example.")
-    parts.append(f"Remove {env_file} from the commit and retry.")
-    print(" ".join(parts))
+        print("Generated .env.example.")
+    print(f"Remove {env_file} from the commit and retry.")


 def main(argv: Sequence[str] | None = None) -> int:
    """Hook entry-point."""
-    parser = argparse.ArgumentParser(description="Block committing environment files (.env).")
+    parser = argparse.ArgumentParser(description="Blocks committing .env files.")
    parser.add_argument('filenames', nargs='*', help='Staged filenames (supplied by pre-commit).')
    parser.add_argument('--create-example', action='store_true', help='Generate example env file (.env.example).')
    args = parser.parse_args(argv)
    env_file = DEFAULT_ENV_FILE
-    # Use current working directory as repository root (simplified; no ascent)
+    # Use current working directory as repository root (pre-commit executes
+    # hooks from the repo root).
    repo_root = os.getcwd()
    gitignore_file = os.path.join(repo_root, DEFAULT_GITIGNORE_FILE)
    example_file = os.path.join(repo_root, DEFAULT_EXAMPLE_ENV_FILE)
--- a/testing/resources/test.env
+++ b/testing/resources/test.env
@ -0,0 +1,82 @@
+# =============================================================================
+# DUMMY SECRETS FOR DOTENV TEST
+# =============================================================================
+
+# Container Internal Ports (what each service listens on inside containers)
+BACKEND_CONTAINER_PORT=3000        # FastAPI server internal port
+FRONTEND_CONTAINER_PORT=3001       # Vite dev server internal port
+
+# External Access (what users/browsers connect to)
+CADDY_EXTERNAL_PORT=80             # External port exposed to host system
+
+# URLs (how different components reference each other)
+BASE_HOSTNAME=http://localhost
+PUBLIC_FRONTEND_URL=${BASE_HOSTNAME}:${CADDY_EXTERNAL_PORT}
+LEGACY_BACKEND_DIRECT_URL=${BASE_HOSTNAME}:${BACKEND_CONTAINER_PORT}  # Deprecated: direct backend access
+VITE_BROWSER_API_URL=${BASE_HOSTNAME}:${CADDY_EXTERNAL_PORT}/api      # Frontend API calls through Caddy
+
+# Environment
+NODE_ENV=development
+# Supabase
+SUPABASE_PROJECT_ID=979090c33e5da06f67921e70
+SUPABASE_PASSWORD=1bbad0861dbca0bad3bd58ac90fd87e1cfd13ebbbeaed730868a11fa38bf6a65
+SUPABASE_URL=https://${SUPABASE_PROJECT_ID}.supabase.co
+DATABASE_URL=postgresql://postgres.${SUPABASE_PROJECT_ID}:${SUPABASE_PASSWORD}@aws-0-us-west-1.pooler.supabase.com:5432/postgres
+SUPABASE_SERVICE_KEY=f37f35e070475d4003ea0973cc15ef8bd9956fd140c80d247a187f8e5b0d69d70a9555decd28ea405051bf31d1d1f949dba277f058ba7c0279359ccdeda0f0696ea803403b8ad76dbbf45c4220b45a44a66e643bf0ca575dffc69f22a57c7d6c693e4d55b5f02e8a0da192065a38b24cbed2234d005661beba6d58e3ef234e0f
+SUPABASE_S3_STORAGE_ENDPOINT=${SUPABASE_URL}/storage/v1/s3
+SUPABASE_STORAGE_BUCKET=my-bucket
+SUPABASE_REGION=us-west-1
+SUPABASE_S3_ACCESS_KEY_ID=323157dcde28202bda94ff4db4be5266
+SUPABASE_S3_SECRET_ACCESS_KEY=d37c900e43e9dfb2c9998fa65aaeea703014504bbfebfddbcf286ee7197dc975
+
+# Storage (aliases for compatibility)
+STORAGE_URL=https://b8991834720f5477910eded7.supabase.co/storage/v1/s3
+STORAGE_BUCKET=my-bucket
+STORAGE_ACCESS_KEY=FEvMws2HMGW96oBMx6Cg98pP8k3h4eki
+STORAGE_SECRET_KEY=shq7peEUeYkdzuUDohoK6qx9Zpjvjq6Zz2coUDvyQARM3qk9QryKZmQqRmz4szzM
+STORAGE_REGION=us-west-1
+STORAGE_SKIP_BUCKET_CHECK=true
+
+# Authentication
+ACCESS_TOKEN_SECRET=ghp_c9d4307ceb82d06b522c1a5e37a8b5d0BMwJpgMT
+REFRESH_TOKEN_SECRET=09cb1b7920aea0d2b63ae3264e27595225ca7132f92f4cc5eff6dc066957118d
+JWT_ALGORITHM=HS256
+
+# Mail
+MAIL_FROM=noreply@example.com
+
+# Chrome Browser
+CHROME_TOKEN=ac126eb015837628b05ff2f0f568ff46
+CHROME_PROXY_HOST=chrome
+CHROME_PROXY_PORT=3002
+CHROME_PROXY_SSL=false
+CHROME_HEALTH=true
+CHROME_PORT=8080
+
+# Test Configuration (for e2e)
+TEST_HOST=${BASE_HOSTNAME}
+TEST_TIMEOUT=35
+TEST_EMAIL=test@example.com
+TEST_PASSWORD=changeme
+POSTGRES_PORT=5432
+MINIO_PORT=9000
+REDIS_PORT=6379
+
+# Database and Storage Paths
+SQLITE_DB_PATH=database.db
+TEST_DB_PATH=tests/testdb.duckdb
+STATIC_FILES_DIR=/app/static
+
+# AI
+OPENAI_API_KEY = "sk-proj-a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0"
+COHERE_API_KEY = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0"
+OR_API_KEY = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0"
+AZURE_API_KEY = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6"
+GEMINI_API_KEY = "AIzaSyA1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r"
+VERTEXAI_API_KEY = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0"
+REPLICATE_API_KEY = "r8_a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9"
+REPLICATE_API_TOKEN = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0"
+ANTHROPIC_API_KEY = "sk-ant-a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0u1v2w3x4y5z6a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6"
+INFISICAL_TOKEN = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0"
+NOVITA_API_KEY = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0"
+INFINITY_API_KEY = "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0"
--- a/tests/catch_dotenv_test.py
+++ b/tests/catch_dotenv_test.py
@ -311,6 +311,8 @@ def test_atomic_write_failure_example(monkeypatch, tmp_path: Path, env_file: Pat
        os.chdir(cwd)
    # hook still blocks; but example creation failed -> message should not claim Example file generated
    assert ok is True
-    out = capsys.readouterr().out
+    captured = capsys.readouterr()
+    out = captured.out
+    err = captured.err
    assert 'Example file generated' not in out
-    assert 'ERROR: unable to write' in out
+    assert 'ERROR: unable to write' in err