Skip to content

Conversation

@nikhilsinhaparseable
Copy link
Contributor

@nikhilsinhaparseable nikhilsinhaparseable commented Jan 7, 2026

Fixes: #1509

Summary by CodeRabbit

  • Chores

    • Updated networking and TLS components and added TLS provider initialization at startup for improved compatibility and reliability.
  • Bug Fixes

    • Safer HTTP header handling when proxying requests to reduce malformed-header errors and improve robustness.
  • Stability

    • Aligned internal HTTP status handling across services to reduce subtle error-mapping inconsistencies.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 7, 2026

Walkthrough

Updated dependencies (rustls 0.23, http 1.0, reqwest 0.12). Replaced many http::StatusCode imports with actix_web::http::StatusCode and adjusted ResponseError::status_code signatures. Switched to rustls provider API, added provider installation in main, and hardened header forwarding into reqwest.

Changes

Cohort / File(s) Summary
Dependency & Manifest
Cargo.toml
Bumped actix-web rustls feature to rustls-0_23, http to 1.0, rustls to 0.23 (features ["ring","std"], default-features=false), and reqwest to 0.12 (default-features=false).
StatusCode Migration (many modules)
src/...
src/alerts/mod.rs, src/correlation.rs, src/handlers/http/*, src/metastore/*, src/metrics/*, src/prism/*, src/utils/*, src/main.rs, ...
Replaced http::StatusCode imports with actix_web::http::StatusCode and updated impl actix_web::ResponseError::status_code return types across modules. Mostly import/type-path changes, no semantic mapping changes.
Header forwarding hardening
src/handlers/http/cluster/mod.rs
Use reqwest::header::HeaderMap and perform guarded conversions from actix headers to reqwest::header::HeaderName/Value, inserting only valid headers to avoid invalid header panics.
Header import adjustments
src/handlers/http/cluster/utils.rs, src/handlers/http/modal/utils/ingest_utils.rs, src/parseable/mod.rs, src/metrics/prom_utils.rs
Aligned header and USER_AGENT imports between http::header and actix_web::http::header as appropriate.
Rustls provider & TLS binding
src/handlers/http/modal/ssl_acceptor.rs, src/handlers/http/modal/mod.rs
Migrate to rustls-0.23 provider-based builder (builder_with_provider) and switch server binding from bind_rustls_0_22 to bind_rustls_0_23.
Crypto provider installation
src/main.rs
Call to install rustls crypto provider at startup with non-fatal warning on failure to prevent runtime panic.
Stream error robustness
src/handlers/http/logstream.rs
More robust extraction of HTTP status from network errors; fallback to INTERNAL_SERVER_ERROR when underlying status unavailable.
Metrics error type update
src/metrics/mod.rs
MetricsError::Custom now holds StatusCode from actix_web::http and updated related signatures/impls.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant ActixServer
    participant Downstream as ReqwestClient
    Note over ActixServer: Actix request handling
    Client->>ActixServer: HTTP request (headers + body)
    ActixServer->>ActixServer: Parse & validate headers
    ActixServer->>ActixServer: Convert headers (guarded)
    alt header conversion OK
        ActixServer->>Downstream: Forward request (reqwest HeaderMap populated)
    else invalid headers skipped
        Note right of ActixServer: Skip invalid header(s)
        ActixServer->>Downstream: Forward request (partial headers)
    end
    Downstream-->>ActixServer: Response (status, headers, body)
    ActixServer-->>Client: Proxy response
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~35 minutes

Possibly related PRs

  • Feat: Metastore #1424 — Modifies ResponseError/status_code imports and handling in src/correlation.rs, closely related to StatusCode import/type-path changes.

Suggested reviewers

  • nitisht
  • parmesant

Poem

🐰
I hopped through crates and swapped a name,
StatusCodes moved without much fame.
I taught rustls how to pick its key,
Guarded headers — now safe and free.
Tidy tunnels, happy spree.

🚥 Pre-merge checks | ✅ 1 | ❌ 4
❌ Failed checks (3 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description only references an issue number without explaining the problem, solution, or key changes, making it incomplete. Add details about the rustls panic issue, explain how the changes fix it (installing CryptoProvider, updating dependencies), and confirm testing of log ingestion and log query.
Out of Scope Changes check ⚠️ Warning The PR contains changes beyond addressing the rustls panic: it systematically updates StatusCode imports from http crate to actix_web::http across 30+ files, which appears to be a separate refactoring effort unrelated to fixing the CryptoProvider issue. Consider splitting this PR into two: one for fixing the rustls panic (dependencies, CryptoProvider installation, TLS acceptor) and another for the StatusCode import refactoring to maintain clear scope and easier review.
Docstring Coverage ⚠️ Warning Docstring coverage is 9.52% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'fix multiple versions of rustls' is vague and does not clearly describe the main change, which is updating rustls to 0.23 and installing a default CryptoProvider. Consider a more descriptive title such as 'Upgrade rustls to 0.23 and install default CryptoProvider to fix startup panic' to better convey the primary changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR successfully addresses the rustls panic by updating rustls from 0.22 to 0.23, updating related dependencies (actix-web, http, reqwest), installing a default CryptoProvider in main.rs, and updating the TLS acceptor to use the provider-based builder.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ab6594 and badd0a5.

📒 Files selected for processing (1)
  • src/main.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main.rs
⏰ 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). (9)
  • GitHub Check: Build Default x86_64-pc-windows-msvc
  • GitHub Check: Build Default x86_64-unknown-linux-gnu
  • GitHub Check: Build Default aarch64-apple-darwin
  • GitHub Check: Build Default aarch64-unknown-linux-gnu
  • GitHub Check: Build Kafka x86_64-unknown-linux-gnu
  • GitHub Check: Build Kafka aarch64-apple-darwin
  • GitHub Check: Quest Smoke and Load Tests for Distributed deployments
  • GitHub Check: coverage
  • GitHub Check: Quest Smoke and Load Tests for Standalone deployments

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @src/main.rs:
- Around line 36-41: The current installation of the rustls crypto provider
(rustls::crypto::ring::default_provider().install_default()) only logs errors
and lets the app continue, which can hide real failures; update the handling to
match on the returned Err, distinguish the "already installed" case from other
errors by inspecting the error variant or message, log a clear message for the
benign "already installed" case and call std::process::exit(1) (or return an Err
from main) for any other fatal error so TLS operations cannot proceed in a
broken state.
🧹 Nitpick comments (1)
src/handlers/http/cluster/mod.rs (1)

371-380: Consider logging failed header conversions for better observability.

The safe header conversion pattern correctly handles incompatible headers by skipping them. However, silently dropping headers could make debugging difficult if critical headers fail to convert.

🔍 Add logging for failed conversions
     for (key, value) in headers.iter() {
         // Convert actix header name/value to reqwest header name/value
         if let Ok(name) = reqwest::header::HeaderName::from_bytes(key.as_str().as_bytes())
             && let Ok(val) = reqwest::header::HeaderValue::from_bytes(value.as_bytes())
         {
             reqwest_headers.insert(name, val);
+        } else {
+            warn!("Failed to convert header '{}' for ingestor sync", key);
         }
     }
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dbb3cb3 and 9daac34.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (32)
  • Cargo.toml
  • src/alerts/mod.rs
  • src/correlation.rs
  • src/handlers/http/cluster/mod.rs
  • src/handlers/http/cluster/utils.rs
  • src/handlers/http/health_check.rs
  • src/handlers/http/ingest.rs
  • src/handlers/http/llm.rs
  • src/handlers/http/logstream.rs
  • src/handlers/http/mod.rs
  • src/handlers/http/modal/ingest/ingestor_logstream.rs
  • src/handlers/http/modal/ingest/ingestor_rbac.rs
  • src/handlers/http/modal/mod.rs
  • src/handlers/http/modal/query/querier_logstream.rs
  • src/handlers/http/modal/ssl_acceptor.rs
  • src/handlers/http/modal/utils/ingest_utils.rs
  • src/handlers/http/oidc.rs
  • src/handlers/http/query.rs
  • src/handlers/http/rbac.rs
  • src/handlers/http/role.rs
  • src/handlers/http/users/dashboards.rs
  • src/handlers/http/users/filters.rs
  • src/main.rs
  • src/metastore/metastores/object_store_metastore.rs
  • src/metastore/mod.rs
  • src/metrics/mod.rs
  • src/metrics/prom_utils.rs
  • src/parseable/mod.rs
  • src/prism/home/mod.rs
  • src/prism/logstream/mod.rs
  • src/utils/error.rs
  • src/utils/header_parsing.rs
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-10-20T17:48:53.444Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1448
File: src/handlers/http/cluster/mod.rs:1370-1400
Timestamp: 2025-10-20T17:48:53.444Z
Learning: In src/handlers/http/cluster/mod.rs, the billing metrics processing logic should NOT accumulate counter values from multiple Prometheus samples with the same labels. The intended behavior is to convert each received counter from nodes into individual events for ingestion, using `.insert()` to store the counter value directly.

Applied to files:

  • src/metrics/prom_utils.rs
  • src/handlers/http/cluster/mod.rs
📚 Learning: 2025-05-01T10:27:56.858Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1305
File: src/handlers/http/users/dashboards.rs:0-0
Timestamp: 2025-05-01T10:27:56.858Z
Learning: The `add_tile()` function in `src/handlers/http/users/dashboards.rs` should use `get_dashboard_by_user(dashboard_id, &user_id)` instead of `get_dashboard(dashboard_id)` to ensure proper authorization checks when modifying a dashboard.

Applied to files:

  • src/handlers/http/users/dashboards.rs
📚 Learning: 2025-09-18T09:52:07.554Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1415
File: src/storage/object_storage.rs:173-177
Timestamp: 2025-09-18T09:52:07.554Z
Learning: In Parseable's upload system (src/storage/object_storage.rs), the update_storage_metrics function can safely use path.metadata().map_err() to fail on local file metadata read failures because parquet validation (validate_uploaded_parquet_file) ensures file integrity before this step, and the system guarantees local staging files remain accessible throughout the upload flow.

Applied to files:

  • src/metastore/mod.rs
📚 Learning: 2025-09-06T04:26:17.191Z
Learnt from: parmesant
Repo: parseablehq/parseable PR: 1424
File: src/enterprise/utils.rs:65-72
Timestamp: 2025-09-06T04:26:17.191Z
Learning: In Parseable's metastore implementation, MetastoreError::to_detail() returns a MetastoreErrorDetail struct (not a string), which contains structured error information including operation, message, stream_name, and other contextual fields. This struct is designed to be boxed in ObjectStorageError::MetastoreError(Box<MetastoreErrorDetail>).

Applied to files:

  • src/metastore/mod.rs
📚 Learning: 2025-08-18T14:56:18.463Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1405
File: src/storage/object_storage.rs:997-1040
Timestamp: 2025-08-18T14:56:18.463Z
Learning: In Parseable's staging upload system (src/storage/object_storage.rs), failed parquet file uploads should remain in the staging directory for retry in the next sync cycle, while successful uploads remove their staged files immediately. Early return on first error in collect_upload_results is correct behavior as concurrent tasks handle their own cleanup and failed files need to stay for retry.

Applied to files:

  • src/metastore/mod.rs
  • src/parseable/mod.rs
📚 Learning: 2025-06-16T04:56:21.613Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1346
File: src/sync.rs:78-83
Timestamp: 2025-06-16T04:56:21.613Z
Learning: In Rust async code, `#[tokio::main]` is appropriate when functions are called from separate OS threads (like `thread::spawn()`), as it creates a new Tokio runtime for that thread. The "Cannot start a runtime from within a runtime" error only occurs when `#[tokio::main]` functions are called from within an existing Tokio runtime context.

Applied to files:

  • src/main.rs
📚 Learning: 2025-10-21T02:22:24.403Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1448
File: src/parseable/mod.rs:419-432
Timestamp: 2025-10-21T02:22:24.403Z
Learning: In Parseable's internal stream creation (`create_internal_stream_if_not_exists` in `src/parseable/mod.rs`), errors should not propagate to fail server initialization. The function creates both pmeta and pbilling internal streams, and failures are logged but the function always returns `Ok(())` to ensure server startup resilience. Individual stream creation failures should not prevent syncing of successfully created streams.

Applied to files:

  • src/handlers/http/logstream.rs
📚 Learning: 2025-09-05T09:18:44.813Z
Learnt from: parmesant
Repo: parseablehq/parseable PR: 1425
File: src/query/mod.rs:484-495
Timestamp: 2025-09-05T09:18:44.813Z
Learning: In the Parseable system, stream names and column names cannot contain quotes, which eliminates SQL injection concerns when interpolating these identifiers directly into SQL queries in src/query/mod.rs.

Applied to files:

  • src/alerts/mod.rs
🧬 Code graph analysis (8)
src/handlers/http/oidc.rs (8)
src/handlers/http/ingest.rs (1)
  • status_code (511-543)
src/handlers/http/rbac.rs (1)
  • status_code (419-439)
src/handlers/http/users/dashboards.rs (1)
  • status_code (264-275)
src/metastore/mod.rs (1)
  • status_code (149-159)
src/prism/home/mod.rs (1)
  • status_code (484-494)
src/prism/logstream/mod.rs (1)
  • status_code (391-404)
src/utils/error.rs (1)
  • status_code (33-35)
src/utils/header_parsing.rs (1)
  • status_code (41-43)
src/handlers/http/ingest.rs (16)
src/alerts/mod.rs (1)
  • status_code (991-1017)
src/correlation.rs (1)
  • status_code (338-350)
src/handlers/http/llm.rs (1)
  • status_code (150-157)
src/handlers/http/logstream.rs (1)
  • status_code (592-628)
src/handlers/http/oidc.rs (1)
  • status_code (566-573)
src/handlers/http/query.rs (1)
  • status_code (593-599)
src/handlers/http/rbac.rs (1)
  • status_code (419-439)
src/handlers/http/role.rs (1)
  • status_code (179-188)
src/handlers/http/users/dashboards.rs (1)
  • status_code (264-275)
src/handlers/http/users/filters.rs (1)
  • status_code (146-155)
src/metastore/mod.rs (1)
  • status_code (149-159)
src/metrics/mod.rs (1)
  • status_code (785-789)
src/prism/home/mod.rs (1)
  • status_code (484-494)
src/prism/logstream/mod.rs (1)
  • status_code (391-404)
src/utils/error.rs (1)
  • status_code (33-35)
src/utils/header_parsing.rs (1)
  • status_code (41-43)
src/handlers/http/role.rs (5)
src/handlers/http/oidc.rs (1)
  • status_code (566-573)
src/handlers/http/query.rs (1)
  • status_code (593-599)
src/metastore/mod.rs (1)
  • status_code (149-159)
src/prism/home/mod.rs (1)
  • status_code (484-494)
src/utils/header_parsing.rs (1)
  • status_code (41-43)
src/handlers/http/llm.rs (15)
src/alerts/mod.rs (1)
  • status_code (991-1017)
src/correlation.rs (1)
  • status_code (338-350)
src/handlers/http/ingest.rs (1)
  • status_code (511-543)
src/handlers/http/oidc.rs (1)
  • status_code (566-573)
src/handlers/http/query.rs (1)
  • status_code (593-599)
src/handlers/http/rbac.rs (1)
  • status_code (419-439)
src/handlers/http/role.rs (1)
  • status_code (179-188)
src/handlers/http/users/dashboards.rs (1)
  • status_code (264-275)
src/handlers/http/users/filters.rs (1)
  • status_code (146-155)
src/metastore/mod.rs (1)
  • status_code (149-159)
src/metrics/mod.rs (1)
  • status_code (785-789)
src/prism/home/mod.rs (1)
  • status_code (484-494)
src/prism/logstream/mod.rs (1)
  • status_code (391-404)
src/utils/error.rs (1)
  • status_code (33-35)
src/utils/header_parsing.rs (1)
  • status_code (41-43)
src/prism/logstream/mod.rs (16)
src/alerts/mod.rs (1)
  • status_code (991-1017)
src/correlation.rs (1)
  • status_code (338-350)
src/handlers/http/ingest.rs (1)
  • status_code (511-543)
src/handlers/http/llm.rs (1)
  • status_code (150-157)
src/handlers/http/logstream.rs (1)
  • status_code (592-628)
src/handlers/http/oidc.rs (1)
  • status_code (566-573)
src/handlers/http/query.rs (1)
  • status_code (593-599)
src/handlers/http/rbac.rs (1)
  • status_code (419-439)
src/handlers/http/role.rs (1)
  • status_code (179-188)
src/handlers/http/users/dashboards.rs (1)
  • status_code (264-275)
src/handlers/http/users/filters.rs (1)
  • status_code (146-155)
src/metastore/mod.rs (1)
  • status_code (149-159)
src/metrics/mod.rs (1)
  • status_code (785-789)
src/prism/home/mod.rs (1)
  • status_code (484-494)
src/utils/error.rs (1)
  • status_code (33-35)
src/utils/header_parsing.rs (1)
  • status_code (41-43)
src/handlers/http/query.rs (15)
src/alerts/mod.rs (1)
  • status_code (991-1017)
src/correlation.rs (1)
  • status_code (338-350)
src/handlers/http/ingest.rs (1)
  • status_code (511-543)
src/handlers/http/llm.rs (1)
  • status_code (150-157)
src/handlers/http/oidc.rs (1)
  • status_code (566-573)
src/handlers/http/rbac.rs (1)
  • status_code (419-439)
src/handlers/http/role.rs (1)
  • status_code (179-188)
src/handlers/http/users/dashboards.rs (1)
  • status_code (264-275)
src/handlers/http/users/filters.rs (1)
  • status_code (146-155)
src/metastore/mod.rs (1)
  • status_code (149-159)
src/metrics/mod.rs (1)
  • status_code (785-789)
src/prism/home/mod.rs (1)
  • status_code (484-494)
src/prism/logstream/mod.rs (1)
  • status_code (391-404)
src/utils/error.rs (1)
  • status_code (33-35)
src/utils/header_parsing.rs (1)
  • status_code (41-43)
src/metrics/mod.rs (3)
src/metastore/mod.rs (1)
  • status_code (149-159)
src/utils/error.rs (1)
  • status_code (33-35)
src/utils/header_parsing.rs (1)
  • status_code (41-43)
src/handlers/http/logstream.rs (5)
src/handlers/http/query.rs (1)
  • status_code (593-599)
src/metastore/mod.rs (1)
  • status_code (149-159)
src/prism/logstream/mod.rs (1)
  • status_code (391-404)
src/utils/error.rs (1)
  • status_code (33-35)
src/utils/header_parsing.rs (1)
  • status_code (41-43)
⏰ 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). (9)
  • GitHub Check: coverage
  • GitHub Check: Build Default x86_64-unknown-linux-gnu
  • GitHub Check: Build Default x86_64-pc-windows-msvc
  • GitHub Check: Build Default aarch64-unknown-linux-gnu
  • GitHub Check: Build Default aarch64-apple-darwin
  • GitHub Check: Build Kafka x86_64-unknown-linux-gnu
  • GitHub Check: Build Kafka aarch64-apple-darwin
  • GitHub Check: Quest Smoke and Load Tests for Standalone deployments
  • GitHub Check: Quest Smoke and Load Tests for Distributed deployments
🔇 Additional comments (34)
src/handlers/http/modal/utils/ingest_utils.rs (1)

20-20: LGTM! Import path correctly updated to use actix-web's re-export.

This change aligns with the dependency upgrade strategy, consolidating imports to use actix_web's HTTP module re-exports instead of directly importing from the http crate. This helps prevent version conflicts when multiple versions of rustls/http dependencies are present in the dependency tree.

src/metrics/prom_utils.rs (1)

27-27: LGTM! Import path updated to use http::header directly.

The change correctly moves the header import from actix_web::http::header to http::header, aligning with the dependency upgrade to http 1.0. The usage at lines 242-243 remains unchanged.

src/handlers/http/modal/ingest/ingestor_logstream.rs (1)

21-21: LGTM! Import path updated to use actix_web::http::StatusCode.

The change correctly moves the StatusCode import to actix_web's http module, which is the appropriate source for framework-specific HTTP status handling. Usage at lines 61 and 86 remains consistent.

src/handlers/http/cluster/utils.rs (1)

25-25: LGTM! Import path updated to use http::header directly.

Consistent with the pattern in other files, moving header utilities to the http crate directly. The usage at line 199 remains unchanged.

src/handlers/http/health_check.rs (1)

21-21: LGTM! Import path updated to use actix_web::http::StatusCode.

The change aligns with the systematic migration to use actix_web's StatusCode type. Usage at lines 40, 121, and 123 remains consistent with the framework's response handling.

src/handlers/http/modal/query/querier_logstream.rs (1)

22-22: LGTM! Import path updated to use actix_web::http::StatusCode.

The change is correct and consistent with the codebase. All StatusCode imports across the codebase use actix_web::http::StatusCode, and the file correctly uses this import at lines 111, 134, and 136.

Note: The header import verification in the original comment had incorrect expectations. The codebase consistently uses actix_web::http::header for all header imports (found in 10+ files), not http::header as suggested. All imports follow a uniform pattern of sourcing from actix_web::http::*.

Likely an incorrect or invalid review comment.

src/handlers/http/mod.rs (1)

21-21: LGTM! Import migration aligned with dependency upgrade.

The StatusCode import has been correctly updated to use actix-web's http module, consistent with the http crate upgrade to 1.0 and the broader migration across the codebase.

src/handlers/http/modal/ingest/ingestor_rbac.rs (1)

21-21: LGTM! Consistent import migration.

The StatusCode import has been correctly migrated to actix-web's http module, consistent with the dependency upgrade.

src/parseable/mod.rs (1)

28-29: LGTM! Import migration completed successfully.

The StatusCode and header-related imports have been correctly migrated from the http crate to actix-web's http module. This change is consistent with the http 1.0 upgrade and aligns with the broader migration pattern across the codebase.

Cargo.toml (1)

34-66: Major version upgrades properly handled — no additional action needed.

The PR upgrades several key dependencies with major version changes (http 0.2.7 → 1.0, rustls 0.22.4 → 0.23, reqwest 0.11.27 → 0.12). The critical breaking changes are properly addressed:

  • rustls 0.23 crypto provider: Correctly initialized via rustls::crypto::ring::default_provider().install_default() in main.rs (lines 36-41) before any TLS operations, with proper error handling.
  • reqwest 0.12 TLS: Explicitly enables "rustls-tls" feature, meeting the requirement that TLS backends must be explicitly configured.
  • http 1.0 API compatibility: No deprecated or problematic API patterns detected; actix-web 4.9.0 ensures header/request/response types are compatible.
src/handlers/http/users/filters.rs (1)

27-27: LGTM! StatusCode import updated correctly.

The import path change from http::StatusCode to actix_web::http::StatusCode aligns with the dependency upgrade objectives. The ResponseError implementation continues to work correctly with the new import.

Also applies to: 146-146

src/handlers/http/cluster/mod.rs (1)

29-30: LGTM! Import changes align with the dependency upgrade.

The import restructuring correctly separates actix-web's HTTP types from the standalone http crate, consistent with upgrading to reqwest 0.12 and the new rustls-based stack.

Also applies to: 34-34

src/utils/error.rs (1)

19-19: LGTM! Import path updated correctly.

The StatusCode import change is consistent with the dependency upgrade to align with actix-web's HTTP module.

src/alerts/mod.rs (1)

19-19: LGTM! Import path updated correctly.

The StatusCode import change aligns with the actix-web HTTP module migration across the codebase.

src/metastore/metastores/object_store_metastore.rs (1)

24-24: LGTM! Import path updated correctly.

The StatusCode import change is consistent with the dependency upgrade and maintains compatibility with existing error handling throughout the file.

src/metastore/mod.rs (1)

19-19: LGTM! StatusCode import migrated to actix-web.

The import change from http::StatusCode to actix_web::http::StatusCode aligns with the dependency upgrade objective and maintains consistent error handling.

src/handlers/http/oidc.rs (2)

21-21: LGTM! Import updated for StatusCode migration.

The import change from http::StatusCode to actix_web::http::StatusCode is consistent with the PR-wide migration pattern.


566-573: LGTM! ResponseError implementation updated correctly.

The status_code() method now returns StatusCode from actix_web::http, maintaining the same error-to-status mappings as before.

src/handlers/http/ingest.rs (2)

21-21: LGTM! Import updated for StatusCode migration.

The import change aligns with the dependency upgrade to use actix_web::http::StatusCode.


511-543: LGTM! ResponseError implementation updated correctly.

The status_code() method now returns StatusCode from actix_web::http. The delegation to MetastoreError::status_code() at line 541 works correctly since MetastoreError was also updated in this PR.

src/handlers/http/role.rs (2)

21-21: LGTM! Import updated for StatusCode migration.

The import change from http::StatusCode to actix_web::http::StatusCode is consistent with the dependency upgrade.


179-188: LGTM! ResponseError implementation updated correctly.

The status_code() method returns StatusCode from actix_web::http, maintaining consistent error mappings. Migration verification confirms no remaining direct http::StatusCode imports across the codebase.

src/handlers/http/modal/mod.rs (1)

153-153: TLS configuration is properly compatible with rustls 0.23. The ssl_acceptor module correctly uses the rustls 0.23 API with the builder-with-provider pattern (ServerConfig::builder_with_provider), custom provider setup, and the appropriate with_safe_default_protocol_versions() method. The configuration returned is fully compatible with the bind_rustls_0_23 method call.

src/correlation.rs (1)

21-21: LGTM! StatusCode import migration is correct.

The migration from http::StatusCode to actix_web::http::StatusCode is consistent with the broader upgrade to consolidate on a single HTTP types source, eliminating the rustls version conflicts mentioned in the PR objectives.

Also applies to: 338-338

src/prism/logstream/mod.rs (1)

21-21: LGTM! Consistent StatusCode migration.

The import and return type changes align with the codebase-wide migration to actix_web::http::StatusCode.

Also applies to: 391-391

src/utils/header_parsing.rs (1)

19-19: LGTM! StatusCode migration is correct.

The import and usage changes are consistent with the project-wide migration to actix_web::http::StatusCode.

Also applies to: 41-42

src/handlers/http/users/dashboards.rs (1)

28-28: LGTM! StatusCode migration is correct.

The import and return type changes align with the codebase-wide consolidation to actix_web::http::StatusCode.

Also applies to: 264-264

src/handlers/http/modal/ssl_acceptor.rs (1)

35-38: Rustls 0.23 migration verified as complete and correct.

All three verification points are confirmed:

  1. ✓ Crypto provider is installed globally via install_default() in src/main.rs:38 (line 38) with proper error handling and explanatory comments
  2. ✓ All rustls usage has been updated to the 0.23 provider-based API; no old ServerConfig::builder() patterns remain in the codebase
  3. ✓ The ring feature is enabled for rustls 0.23 in Cargo.toml with ["ring", "std"]

The code at lines 35-38 correctly uses builder_with_provider() with safe default protocol versions and no client authentication. The migration is complete.

src/prism/home/mod.rs (1)

21-21: LGTM! Import migration aligns with the dependency unification.

The switch from http::StatusCode to actix_web::http::StatusCode is consistent with the PR's goal of resolving multiple rustls versions by consolidating the HTTP type dependencies.

Also applies to: 484-484

src/handlers/http/rbac.rs (1)

32-32: LGTM! StatusCode import aligned with actix-web's HTTP module.

The change correctly updates the import path while preserving all existing status code mappings for the RBACError variants.

Also applies to: 419-419

src/handlers/http/query.rs (1)

25-25: LGTM! Import migration is consistent with the codebase-wide refactor.

The QueryError ResponseError implementation correctly uses the updated StatusCode type from actix_web::http.

Also applies to: 593-593

src/handlers/http/llm.rs (1)

19-21: LGTM! Correct separation of StatusCode and header imports.

The StatusCode is now sourced from actix_web::http, while http::header is retained for the reqwest client's header constant on line 109. This is the correct approach since reqwest expects http crate types for request building.

Also applies to: 150-150

src/handlers/http/logstream.rs (1)

509-509: LGTM! Safe status code conversion for network errors.

The handling of StreamError::Network correctly converts reqwest's StatusCode (from the http crate) to actix-web's StatusCode using from_u16(s.as_u16()). This pattern safely bridges the two types while properly falling back to INTERNAL_SERVER_ERROR if conversion fails.

Also applies to: 592-592, 618-621

src/metrics/mod.rs (1)

762-765: LGTM! Consistent StatusCode migration in metrics module.

The MetricsError type and its ResponseError implementation correctly use actix_web::http::StatusCode. The fully qualified path at line 764 is acceptable since the usage is in the parent module scope.

Also applies to: 775-775, 785-785

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @src/main.rs:
- Around line 36-41: Move the rustls provider installation to after
init_logger() so logs are visible, i.e., call
rustls::crypto::ring::default_provider().install_default() only once the logger
is initialized (after init_logger()). Replace the blind warn! with explicit
handling: match the install_default() result and treat the "already installed"
case as benign (log at debug/info) while treating other Err variants as real
failures (log error via error! with the error details and terminate startup via
std::process::exit(1)); if the exact "already installed" variant/type isn’t
accessible, fall back to checking the error string for an "already installed"
marker but prefer matching the concrete error variant from install_default().
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9daac34 and 1ab6594.

📒 Files selected for processing (1)
  • src/main.rs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-16T04:56:21.613Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1346
File: src/sync.rs:78-83
Timestamp: 2025-06-16T04:56:21.613Z
Learning: In Rust async code, `#[tokio::main]` is appropriate when functions are called from separate OS threads (like `thread::spawn()`), as it creates a new Tokio runtime for that thread. The "Cannot start a runtime from within a runtime" error only occurs when `#[tokio::main]` functions are called from within an existing Tokio runtime context.

Applied to files:

  • src/main.rs
⏰ 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). (9)
  • GitHub Check: Quest Smoke and Load Tests for Standalone deployments
  • GitHub Check: Quest Smoke and Load Tests for Distributed deployments
  • GitHub Check: Build Default x86_64-pc-windows-msvc
  • GitHub Check: coverage
  • GitHub Check: Build Default aarch64-unknown-linux-gnu
  • GitHub Check: Build Default x86_64-unknown-linux-gnu
  • GitHub Check: Build Default aarch64-apple-darwin
  • GitHub Check: Build Kafka aarch64-apple-darwin
  • GitHub Check: Build Kafka x86_64-unknown-linux-gnu

@nitisht nitisht merged commit 31a3ad9 into parseablehq:main Jan 7, 2026
12 checks passed
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.

Parseable not running starting 2.5.4 and also with 2.5.5 now

2 participants