src: handle nameless user @ ProcessMetrics::Update

[santigimeno]

If nsolid is running as a user without a name (e.g. system user), uv_os_get_passwd may fail. This change ensures ProcessMetrics::Update still fills metrics and does not return early in that case, leaving the user field empty instead of failing the update.

Summary by CodeRabbit

• New Features
  • None.
• Bug Fixes
  • Improved resilience of process metrics collection: failures to fetch process title or user no longer stop the update cycle.
  • Metrics now continue to refresh with partial data when lookups fail, avoiding gaps in reporting.
  • The user or title fields may be blank if the system cannot provide them, instead of causing errors.
• Refactor
  • Simplified control flow to reduce early exits during metrics updates.

[coderabbitai]

Walkthrough

Updates ProcessMetrics::Update() in src/nsolid.cc to avoid early returns on failures from uv_get_process_title and uv_os_get_passwd. Introduces local title and user variables, conditionally creates the OnScopeLeave guard and captures user on successful passwd lookup, and continues updating metrics with partial data when lookups fail.

Changes

Cohort / File(s)	Summary
Process metrics update flow src/nsolid.cc	- Added local title and user variables in ProcessMetrics::Update(). - uv_get_process_title: assign to local title only on er == 0; no early return on error. - uv_os_get_passwd: create OnScopeLeave guard and set user only on er == 0; on error, skip guard and leave user empty. - stor_.title and stor_.user are now assigned from local title and user (allowing partial data when lookups fail).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant PM as ProcessMetrics::Update
  participant UV as libuv
  participant Stor as stor_

  PM->>UV: uv_get_process_title(title_buf)
  alt success (er == 0)
    PM->>PM: title = title_buf
  else failure
    PM->>PM: leave title empty, continue
  end

  PM->>UV: uv_os_get_passwd(&pwd)
  alt success (er == 0)
    rect rgba(200,235,255,0.3)
    note right of PM: create OnScopeLeave guard
    PM->>PM: user = pwd.username
    end
  else failure
    PM->>PM: leave user empty, no guard
  end

  PM->>Stor: stor_.title = title
  PM->>Stor: stor_.user = user
  PM->>PM: continue updating remaining metrics

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hop through bytes with tidy paws,
No crash, no halt, just graceful laws.
If title hides or user flees,
I keep the rest — no frantic pleas.
Partial logs, yet steady cheer,
A rabbit guard keeps metrics near. 🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "src: handle nameless user @ ProcessMetrics::Update" succinctly identifies the primary change—making ProcessMetrics::Update tolerate a missing passwd/name—so it accurately reflects the main change in the diff. It names the module and the specific function, which helps reviewers quickly understand the intent. The phrasing is concise and directly related to the PR objectives and file modifications.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch santi/fill_metrics_regardless

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/nsolid.cc (2)

214-214: Add an intent-revealing comment for the empty-user fallback.

A brief note here will prevent future regressions and clarifies why an empty username is acceptable.

+  // uv_os_get_passwd() may fail (e.g., nameless system user). Leave `user` empty and continue.
   std::string user;

---

246-246: Avoid an extra string copy.

Move-assign the local into the stored field.

-  stor_.user = user;
+  stor_.user = std::move(user);

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a296caf and 4d54b70.

📒 Files selected for processing (1)

• src/nsolid.cc (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/nsolid.cc (1)

deps/uv/src/uv-common.c (1)

• uv_os_free_passwd (132-147)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

• GitHub Check: coverage-windows
• GitHub Check: test-linux (ubuntu-24.04)
• GitHub Check: test-macOS
• GitHub Check: lint-js-and-md
• GitHub Check: test-linux (ubuntu-24.04-arm)
• GitHub Check: build-tarball

🔇 Additional comments (1)

src/nsolid.cc (1)

235-238: Good: continue metrics when passwd lookup fails; RAII frees pwd safely.

This matches the PR objective and avoids an unnecessary early return. The scope guard correctly frees pwd on success.

Please confirm no downstream consumer assumes stor_.user is non-empty (UI labels, logs, or filters). If any do, they should handle "" gracefully.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dc4953e and ff5c452.

📒 Files selected for processing (1)

• src/nsolid.cc (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/nsolid.cc (1)

deps/uv/src/uv-common.c (1)

• uv_os_free_passwd (132-147)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

• GitHub Check: test-macOS
• GitHub Check: build-tarball
• GitHub Check: test-linux (ubuntu-24.04-arm)
• GitHub Check: test-linux (ubuntu-24.04)
• GitHub Check: coverage-windows
• GitHub Check: lint-js-and-md

🔇 Additional comments (1)

src/nsolid.cc (1)

241-244: Good: passwd failure no longer aborts metrics; memory is freed conditionally

Proceeding when uv_os_get_passwd() fails and guarding uv_os_free_passwd(&pwd) with OnScopeLeave only on success is correct and avoids freeing uninitialized memory. Setting user to empty is aligned with the PR goal.

Please confirm no downstream consumers assume stor_.user is non-empty (e.g., schema validators, dashboards). If unsure, I can provide a quick repo scan script to flag such assumptions.

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (1)

src/nsolid.cc (1)

251-252: Resolved UB risk: assign from guarded locals.

Assigning stor_.title/stor_.user from title/user avoids reading from potentially uninitialized buffers.

🧹 Nitpick comments (1)

src/nsolid.cc (1)

214-225: Graceful fallback for process title — LGTM; consider UV_ENOBUFS best-effort.

Good move to avoid early return and to guard the assignment via a safe std::string. Optionally, if uv_get_process_title() populates a truncated buffer on UV_ENOBUFS, you could still keep a best‑effort title:

- er = uv_get_process_title(title_buf, sizeof(title_buf));
- if (er == 0) {
+ er = uv_get_process_title(title_buf, sizeof(title_buf));
+ if (er == 0 || er == UV_ENOBUFS) {
   title = title_buf;
 }

Validate libuv’s behavior before adopting.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ff5c452 and b6ab935.

📒 Files selected for processing (1)

• src/nsolid.cc (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

• GitHub Check: test-linux (ubuntu-24.04-arm)
• GitHub Check: test-linux (ubuntu-24.04)
• GitHub Check: test-macOS
• GitHub Check: build-tarball
• GitHub Check: lint-js-and-md
• GitHub Check: coverage-windows

🔇 Additional comments (1)

src/nsolid.cc (1)

241-245: passwd lookup handled correctly — no downstream non-empty user assumptions found.

Searched consumers/tests referencing the metric; only usages propagate or validate presence (test/common/nsolid-metrics-util.js, test/common/nsolid-grpc-agent/index.js) and none assert a non-empty string — no change required.

[santigimeno]

Landed in a296caf...ae1497d