You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Initialization and teardown result handling libs/et-otlp/src/lib.rs, libs/test-helpers/src/lib.rs, services/ws-pyo3-runner/tests/modules.rs
Logger initialization errors now propagate, while shutdown, process, thread, and stderr results are captured without changing best-effort teardown behavior.
WebSocket runtime result handling services/ws-pyo3-runner/..., services/ws-test-server/tests/helpers.rs, services/ws-wasi-runner/src/host/ws.rs, services/ws-wasm-agent/tests/web.rs, services/ws/src/lib.rs
WebSocket channel, task, worker, socket, and promise results are explicitly captured; the scripted server task remains detached.
Constants and derived defaults libs/edge-toolkit/src/config.rs, libs/path/src/lib.rs, services/ws-modules/har1/src/lib.rs, services/ws-modules/sensor1/src/lib.rs
Environment-variable literals are named with local constants, and both DeviceSensors types use derived Default implementations.
We reviewed changes in 5632e3e...af888ec on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR successfully addresses multiple DeepSource bug-risk alerts by modernizing trait implementations and improving Result handling. Codacy analysis indicates the changes are up to standards with no new issues introduced. However, a significant functional change was identified in the OTLP initialization logic (libs/et-otlp/src/lib.rs) where the code now propagates errors instead of silently ignoring them. While this improves robustness, it may cause startup failures if the application environment relies on existing, partially initialized global loggers. Furthermore, a logic issue in the test teardown (services/ws-pyo3-runner/tests/modules.rs) introduces a race condition that could lead to flaky CI runs. The PR description is currently empty, which prevents proper tracking of the specific alerts being resolved.
About this PR
The shift from silent failure to error propagation in global logger initialization is a systemic change. Ensure that all entry points calling this initialization are prepared to handle a Result error, particularly in environments where a logger might already be configured.
The PR description is empty. Please provide the specific DeepSource alert IDs or a summary of the bug-risk categories being addressed to ensure all intended fixes are covered.
Test suggestions
Verify that LogTracer::init handles double-initialization or configuration errors by propagating the error as requested in the change.
Ensure DeviceSensors::default() correctly initializes all fields to the expected state (false/None) via the derived implementation.
Verify that mise_env_includes correctly reads the 'MISE_ENV' environment variable using the new constant.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify that LogTracer::init handles double-initialization or configuration errors by propagating the error as requested in the change.
2. Ensure DeviceSensors::default() correctly initializes all fields to the expected state (false/None) via the derived implementation.
3. Verify that mise_env_includes correctly reads the 'MISE_ENV' environment variable using the new constant.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
This unwrap() introduces a race condition that can cause flaky tests if the runner exits just before kill() is called. Since wait() follows immediately, you can safely ignore the result of kill() to avoid unnecessary panics if the process is already dead.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
Suggestion: Swallowing the join result hides panics in the dispatcher thread. Propagating the panic ensures that the agent fails visibly if the Python worker crashes.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
Suggestion: Changed from silent ignore (drop) to error propagation (?). While this is a robustness improvement that prevents running with broken telemetry, it will cause initialization to fail if a global logger was already set elsewhere. Verify that callers are prepared to handle this change in return type.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/ws-pyo3-runner/src/agent.rs`:
- Around line 159-169: Update the dispatch flow around the Connect and Shutdown
sends in run() to propagate inbound_tx.send failures instead of ignoring them,
while preserving shutdown ordering. Also propagate worker.join() failures so
worker termination or panic causes run() to return an error rather than
appearing successful.
In `@services/ws/src/lib.rs`:
- Around line 331-337: Update the direct-message delivery path around
recipient.send in the AgentMessage handling flow so its Result is checked rather
than discarded. Only report or count delivery as successful when send succeeds,
and handle failed sends consistently with the existing delivery-status behavior,
including broadcast delivery where applicable.
🪄 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 Plus
Run ID: c0873c4a-e108-4dec-8079-d61eaba2edfc
📥 Commits
Reviewing files that changed from the base of the PR and between 5632e3e and af888ec.
sed -n '1,220p' services/ws-pyo3-runner/src/error.rs
Repository: edge-toolkit/core
Length of output: 1160
Propagate dispatch-worker failure instead of silently dropping events. If inbound_tx.send(...) fails, the Python worker is already gone, but run() keeps the socket alive and inbound frames are lost. Surface that error, and do the same for worker.join() so a worker panic does not look like a successful run.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/ws-pyo3-runner/src/agent.rs` around lines 159 - 169, Update the
dispatch flow around the Connect and Shutdown sends in run() to propagate
inbound_tx.send failures instead of ignoring them, while preserving shutdown
ordering. Also propagate worker.join() failures so worker termination or panic
causes run() to return an error rather than appearing successful.
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
# Show the relevant file and line ranges.
sed -n '300,360p' services/ws/src/lib.rs
printf'\n====\n'
sed -n '500,540p' services/ws/src/lib.rs
printf'\n====\n'# Find delivery/status related enums or message variants in the same file.
rg -n "Delivered|Broadcast|Direct|send\\(" services/ws/src/lib.rs
Repository: edge-toolkit/core
Length of output: 6305
🏁 Script executed:
set -euo pipefail
printf'Need tool output\n'
Repository: edge-toolkit/core
Length of output: 172
Don't mark direct or broadcast delivery as successful when send can fail.recipient.send(...) is ignored here, so a disconnect after the registry snapshot can still produce a false Delivered/broadcast status. Handle the error or count only successful sends.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/ws/src/lib.rs` around lines 331 - 337, Update the direct-message
delivery path around recipient.send in the AgentMessage handling flow so its
Result is checked rather than discarded. Only report or count delivery as
successful when send succeeds, and handle failed sends consistently with the
existing delivery-status behavior, including broadcast delivery where
applicable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary by CodeRabbit
Bug Fixes
Refactor