Skip to content

cli: Log data dir before error#252

Open
turuslan wants to merge 1 commit intolambdaclass:mainfrom
turuslan:cli/data-dir
Open

cli: Log data dir before error#252
turuslan wants to merge 1 commit intolambdaclass:mainfrom
turuslan:cli/data-dir

Conversation

@turuslan
Copy link

Log absolute data dir before error to see directory path that causes error.

+2026-03-26T01:51:32.188407Z  INFO ethlambda: Opening data directory data_dir=/app/data
 Failed to create data directory: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }

To see directory path that causes error
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 26, 2026

Greptile Summary

This PR adds a single info! log line that resolves and prints the absolute data directory path immediately before the create_dir_all call, making permission-denied and similar startup errors much easier to diagnose (especially in containerized environments where the working directory may be unexpected).

  • Adds info!(data_dir = %, "Initializing DB") at line 140, using std::path::absolute with a fallback to the original path on failure.
  • The change is logically correct: the temporary PathBuf from unwrap_or_else lives until the end of the statement, so the Display borrow is safe.
  • Minor inconsistency: the existing "Initialized DB" log immediately after (line 144) still displays the relative path, whereas the new log uses the absolute path.

Confidence Score: 5/5

Safe to merge — single-line diagnostic log addition with no behavioral changes.

The change is minimal and non-functional: it adds one log statement before an existing create_dir_all call. The absolute-path resolution is done correctly with a safe fallback. The only observation is a minor inconsistency with the existing follow-up log line, which is a non-blocking P2 style suggestion.

No files require special attention.

Important Files Changed

Filename Overview
bin/ethlambda/src/main.rs Adds an info! log of the resolved absolute data directory path before create_dir_all and RocksDBBackend::open, improving diagnosability of startup permission errors. One minor inconsistency: the follow-up "Initialized DB" log still uses the relative path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[parse CLI options] --> B[read_validator_keys]
    B --> C["info!: Initializing DB\n(absolute data_dir) NEW"]
    C --> D["create_dir_all(data_dir)"]
    D -->|PermissionDenied / other error| E["panic: Failed to create data directory"]
    D -->|OK| F["RocksDBBackend::open(data_dir)"]
    F -->|error| G["panic: Failed to open RocksDB"]
    F -->|OK| H["info!: Initialized DB\n(relative data_dir)"]
    H --> I[fetch_initial_state / rest of startup]
Loading

Comments Outside Diff (1)

  1. bin/ethlambda/src/main.rs, line 144 (link)

    P2 Inconsistent path format between log lines

    The new "Initializing DB" log at line 140 resolves the absolute path (helpful for diagnosing errors), but the existing "Initialized DB" log immediately below still uses the raw relative path from options.data_dir. For consistency — and so that both log entries are equally useful in production — consider using the same absolute-path resolution here.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: bin/ethlambda/src/main.rs
    Line: 144
    
    Comment:
    **Inconsistent path format between log lines**
    
    The new "Initializing DB" log at line 140 resolves the absolute path (helpful for diagnosing errors), but the existing "Initialized DB" log immediately below still uses the raw relative path from `options.data_dir`. For consistency — and so that both log entries are equally useful in production — consider using the same absolute-path resolution here.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix All With AI
This is a comment left during a code review.
Path: bin/ethlambda/src/main.rs
Line: 144

Comment:
**Inconsistent path format between log lines**

The new "Initializing DB" log at line 140 resolves the absolute path (helpful for diagnosing errors), but the existing "Initialized DB" log immediately below still uses the raw relative path from `options.data_dir`. For consistency — and so that both log entries are equally useful in production — consider using the same absolute-path resolution here.

```suggestion
    info!(data_dir = %std::path::absolute(&options.data_dir).unwrap_or_else(|_| options.data_dir.clone()).display(), "Initialized DB");
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "Log absolute data dir before error" | Re-trigger Greptile

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant