Conversation
github-actions
Bot
added
rust
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 24 minutes and 43 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/core/src/commands/mod.rs (1)
189-196: The first field assumption is correct per the Nix daemon protocol, but silent errors mask issues.Line 194's assumption that
fields[0]isField::Stringis valid: the Nix daemon protocol specifies thatresBuildLogLine(ResultType=101) contains exactly one String field with the log text. However, lines 195 and 202–205 silently drop log lines without any warning—either when the field is missing, not a String, or when UTF-8 decoding fails. This obscures failures that should be logged.Consider emitting a debug or warn log when BuildLogLine parsing fails, so issues in the upstream logger become visible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/core/src/commands/mod.rs` around lines 189 - 196, The LogMessage::Result match arm handling ResultType::BuildLogLine currently silently returns None when the first field is missing/not a String or when UTF-8 decoding fails; update the LogMessage::Result { r#type: ResultType::BuildLogLine, fields, .. } branch to log a debug or warn (e.g., using tracing::warn! or crate logger) whenever fields.into_iter().next() is not Field::String or when String::from_utf8/str::from_utf8 fails, including contextual info (the actual Field variant or the decoding error) and still return None; reference the LogMessage::Result match arm and ResultType::BuildLogLine to locate the code to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/core/src/commands/mod.rs`:
- Around line 198-206: The code currently drops lines when UTF-8 decoding fails
by mapping to .ok(); instead, preserve the line using a lossy fallback: replace
the match that builds msg so invalid UTF-8 is converted with
String::from_utf8_lossy (producing a Cow<str>) instead of returning None. Update
the bytes branch to use
str::from_utf8(bytes).map(Cow::Borrowed).unwrap_or_else(|_|
String::from_utf8_lossy(bytes).into()) and the vec branch to use
String::from_utf8(vec).map(Cow::Owned).unwrap_or_else(|e|
String::from_utf8_lossy(&e.into_bytes()).into()), leaving the subsequent match
that yields (msg, VerbosityLevel::Info) intact.
---
Nitpick comments:
In `@crates/core/src/commands/mod.rs`:
- Around line 189-196: The LogMessage::Result match arm handling
ResultType::BuildLogLine currently silently returns None when the first field is
missing/not a String or when UTF-8 decoding fails; update the LogMessage::Result
{ r#type: ResultType::BuildLogLine, fields, .. } branch to log a debug or warn
(e.g., using tracing::warn! or crate logger) whenever fields.into_iter().next()
is not Field::String or when String::from_utf8/str::from_utf8 fails, including
contextual info (the actual Field variant or the decoding error) and still
return None; reference the LogMessage::Result match arm and
ResultType::BuildLogLine to locate the code to change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f9d1b3f-754b-49ff-9f4b-96ec9ac04483
📒 Files selected for processing (1)
crates/core/src/commands/mod.rs
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/core/src/commands/mod.rs (1)
200-210:⚠️ Potential issue | 🟡 MinorUse lossy UTF-8 fallback instead of dropping build-log lines.
Line 202 and Line 204 convert decode failures to
None, and Line 209 drops the message. That loses potentially important diagnostics; preserve the line withfrom_utf8_lossyinstead.Suggested fix
- let msg = match msg { - std::borrow::Cow::Borrowed(bytes) => { - str::from_utf8(bytes).map(Cow::Borrowed).ok() - } - std::borrow::Cow::Owned(vec) => String::from_utf8(vec).map(Cow::Owned).ok(), - }; - - match msg { - Some(msg) => (msg, VerbosityLevel::Info), - None => return None, - } + let msg = match msg { + std::borrow::Cow::Borrowed(bytes) => { + str::from_utf8(bytes) + .map(Cow::Borrowed) + .unwrap_or_else(|_| String::from_utf8_lossy(bytes)) + } + std::borrow::Cow::Owned(vec) => match String::from_utf8(vec) { + Ok(s) => Cow::Owned(s), + Err(e) => String::from_utf8_lossy(&e.into_bytes()), + }, + }; + + (msg, VerbosityLevel::Info)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/core/src/commands/mod.rs` around lines 200 - 210, The current match on msg (handling std::borrow::Cow variants) discards lines when UTF-8 decoding fails because str::from_utf8(...) and String::from_utf8(...) map errors to None, causing the function to return None; instead decode using from_utf8_lossy so invalid UTF-8 is replaced with the Unicode replacement char and the line is preserved. Update the Borrowed branch to use str::from_utf8_lossy(bytes) and the Owned branch to use String::from_utf8_lossy(vec) (or convert Owned vec into a lossy String), and keep the subsequent match returning (msg, VerbosityLevel::Info) so malformed bytes become lossy Strings rather than being dropped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/core/src/commands/mod.rs`:
- Around line 200-210: The current match on msg (handling std::borrow::Cow
variants) discards lines when UTF-8 decoding fails because str::from_utf8(...)
and String::from_utf8(...) map errors to None, causing the function to return
None; instead decode using from_utf8_lossy so invalid UTF-8 is replaced with the
Unicode replacement char and the line is preserved. Update the Borrowed branch
to use str::from_utf8_lossy(bytes) and the Owned branch to use
String::from_utf8_lossy(vec) (or convert Owned vec into a lossy String), and
keep the subsequent match returning (msg, VerbosityLevel::Info) so malformed
bytes become lossy Strings rather than being dropped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0ae6f90-b725-4682-aa6f-9bcd6ae48a28
📒 Files selected for processing (1)
crates/core/src/commands/mod.rs
mrshmllow
force-pushed
the
marshmallow/p-kqrvwpxkulmm
branch
from
541a4f7 to
59dff93
Compare
mrshmllow
force-pushed
the
marshmallow/p-kqrvwpxkulmm
branch
from
59dff93 to
733b954
Compare
1 participant
Summary by CodeRabbit