Prepare for Hyper 1.0.0 Upgrade: Enable Deprecated Features and Resolve Deprecations #1938

djnovin · 2024-12-28T10:54:20Z

This change allows us to address deprecation warnings without immediately migrating to the newer Hyper 1.0.0 APIs. A full upgrade will be planned in subsequent PRs.

Description of change

This PR prepares the codebase for upgrading Hyper from version 0.14 to 1.0.0 by addressing deprecation warnings and enabling compatibility with newer APIs. Key changes include:

Enabled the deprecated feature flag in Hyper 0.14 to resolve immediate deprecation warnings.
Refactored usages of deprecated APIs (e.g., aggregate and to_bytes) to align with Hyper's updated patterns.
Ensured tests and functionality remain intact with the current version while paving the way for the full migration to Hyper 1.0.0.

These changes ensure that the upgrade process is smoother and minimize risks of breaking changes in subsequent steps.

#1517

How has this been tested? (if applicable)

cargo test has been run...
Deprecated API's updated in tests...

- Enabled the `deprecated` and `backports` feature flag in Hyper 0.14 to resolve deprecation warnings. - Temporarily addresses warning issues with older `to_bytes` api. - Ensures the codebase remains functional while preparing for a full upgrade to Hyper 1.0.0. This change allows us to address deprecation warnings without immediately migrating to the newer Hyper 1.0.0 APIs. A full upgrade will be planned in subsequent commits.

greptile-apps

PR Summary

Let me provide a concise summary of the key changes in this PR:

Replaces deprecated hyper::body::to_bytes() with hyper::body::HttpBody::collect().await.unwrap().to_bytes() across multiple files to prepare for Hyper 1.0.0 upgrade
Enables 'backports' and 'deprecated' features in Hyper 0.14.23 dependency to maintain compatibility during transition
Introduces potential unwrap() issues in some error handling paths that should be reviewed
Changes are primarily in test code and HTTP body handling, maintaining existing functionality while using newer API patterns

Here are the 3 most important points to focus on:

The use of unwrap() after collect() in several files like provisioner_server.rs could lead to panics if body collection fails - consider using proper error handling instead.
The PR enables deprecated features as a temporary solution, but a follow-up PR will be needed for the full Hyper 1.0.0 migration as noted in issue [Improvement]: Bump shuttle-tower service to hyper 1.1.0 #1517.
The changes are focused on HTTP body handling patterns and don't modify core business logic, making this a relatively safe preparation step for the larger upgrade.

_{💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!}

_{15 file(s) reviewed, 13 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2024-12-28T10:54:59Z

auth/tests/api/auth.rs

-        let body = hyper::body::to_bytes(response.into_body()).await.unwrap();
+        let body = hyper::body::HttpBody::collect(response.into_body())
+            .await
+            .unwrap()


style: consider handling the unwrap() case more gracefully since this is a test that could fail

greptile-apps · 2024-12-28T10:55:01Z

auth/tests/api/helpers.rs

+        let public_key = hyper::body::HttpBody::collect(response.into_body())
+            .await
+            .unwrap()
+            .to_bytes();


style: consider extracting body collection logic into a helper method to avoid repetition

greptile-apps · 2024-12-28T10:55:03Z

auth/tests/api/users.rs

+        let body = hyper::body::HttpBody::collect(response.into_body())
+            .await
+            .unwrap()
+            .to_bytes();


style: This pattern is repeated throughout the file. Consider extracting this common body collection logic into a helper function to reduce duplication and make future API changes easier.

greptile-apps · 2024-12-28T10:55:05Z

Cargo.toml

 # not great, but waiting for WebSocket changes to be merged
 hyper-reverse-proxy = { git = "https://github.com/chesedo/hyper-reverse-proxy", branch = "bug/host_header" }


style: Using a git dependency for hyper-reverse-proxy could cause build instability. Consider pinning to a specific commit hash rather than branch, or better yet, wait for the WebSocket changes to be merged and use a released version.

greptile-apps · 2024-12-28T10:55:29Z

backends/src/axum.rs

+        let body = hyper::body::HttpBody::collect(response.into_body())
+            .await
+            .unwrap()
+            .to_bytes();


style: consider extracting this repeated body collection pattern into a helper function to reduce duplication

greptile-apps · 2024-12-28T10:56:04Z

deployer/src/handlers/local.rs

        let convert: Value = serde_json::from_slice(&body)
            .expect("failed to deserialize body as JSON, did you login?");


style: The error message 'did you login?' may be misleading since this could fail for other reasons like invalid JSON. Consider a more generic error message.

greptile-apps · 2024-12-28T10:56:11Z

gateway/src/api/latest.rs

+    let body = hyper::body::HttpBody::collect(res.body_mut())
+        .await
+        .unwrap()
+        .to_bytes();


style: Consider handling unwrap() here with proper error propagation since this is in the request path

greptile-apps · 2024-12-28T10:56:12Z

gateway/src/api/auth_layer.rs

+                        let body = match hyper::body::HttpBody::collect(token_response.into_body())
+                            .await
+                        {
+                            Ok(aggregated_body) => aggregated_body.to_bytes(),
+                            Err(err) => {


style: The new collect().await?.to_bytes() pattern could potentially use more memory than the previous to_bytes() since it aggregates the entire body before converting to bytes. Consider adding a size limit check.

greptile-apps · 2024-12-28T10:56:47Z

gateway/src/service.rs

-                .unwrap_or_default();
+                .unwrap_or_default()
+                .to_bytes();
+
            let convert: serde_json::Value = serde_json::from_slice(&body).unwrap_or_default();


style: Consider adding error handling for JSON parsing failures rather than using unwrap_or_default

greptile-apps · 2024-12-28T10:56:48Z

gateway/src/project.rs

+            let body = hyper::body::HttpBody::collect(resp.into_body())
+                .await?
+                .to_bytes();


style: Consider handling potential memory issues by adding a size limit to body collection

oddgrd · 2024-12-31T13:18:57Z

Hello @djnovin, thanks for the PR! This looks great, and it seems like you've gone about starting the upgrade in a reasonable and correct way. While we do want to upgrade hyper and associated crates, a lot of the services in this repository are about to be deprecated at the end of January (e.g. auth, gateway and deployer), replaced by services on the new platform: https://docs.shuttle.dev/platform-update/platform-update. Those new services are closed-source for the time being, but they may be opened up in the future.

Still, we do want to update the shuttle-runtime crate, shared crates like shuttle-common, and also the library referenced in the ticket: #1517. It may be difficult to do as an external contributor without access to the new services, however. We'll discuss this internally when the rest of the team is back after the holidays, and we'll get back to you if we think this is suitable for an external contributor to work on. Happy new year!

oddgrd · 2025-01-09T12:00:29Z

Hello again @djnovin! We've discussed this internally, and decided that we'd rather wait with this upgrade until the old version of the platform has been fully deprecated. That is planned for the end of January. There will then be some cross-cutting work between these public crates and the code in our currently private repos, so it may still be a bit tricky for an external contributor to work. However, if we do think it's suitable for you to work on, we'll let you know in case you're still interested at the time.

And again, thank you for the contribution, it looks great, and had it not been for the ongoing platform switch, we would have been able to merge it quickly. We aim to open more issues for contributors once the new version of the platform is more settled, so we hope to see you again then!

greptile-apps bot reviewed Dec 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare for Hyper 1.0.0 Upgrade: Enable Deprecated Features and Resolve Deprecations #1938

Prepare for Hyper 1.0.0 Upgrade: Enable Deprecated Features and Resolve Deprecations #1938

djnovin commented Dec 28, 2024

greptile-apps bot left a comment

greptile-apps bot Dec 28, 2024

greptile-apps bot Dec 28, 2024

greptile-apps bot Dec 28, 2024

greptile-apps bot Dec 28, 2024

greptile-apps bot Dec 28, 2024

greptile-apps bot Dec 28, 2024

greptile-apps bot Dec 28, 2024

greptile-apps bot Dec 28, 2024

greptile-apps bot Dec 28, 2024

greptile-apps bot Dec 28, 2024

oddgrd commented Dec 31, 2024

oddgrd commented Jan 9, 2025

		# not great, but waiting for WebSocket changes to be merged
		hyper-reverse-proxy = { git = "https://github.com/chesedo/hyper-reverse-proxy", branch = "bug/host_header" }

		let convert: Value = serde_json::from_slice(&body)
		.expect("failed to deserialize body as JSON, did you login?");

Prepare for Hyper 1.0.0 Upgrade: Enable Deprecated Features and Resolve Deprecations #1938

Are you sure you want to change the base?

Prepare for Hyper 1.0.0 Upgrade: Enable Deprecated Features and Resolve Deprecations #1938

Conversation

djnovin commented Dec 28, 2024

Description of change

How has this been tested? (if applicable)

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Dec 28, 2024

Choose a reason for hiding this comment

greptile-apps bot Dec 28, 2024

Choose a reason for hiding this comment

greptile-apps bot Dec 28, 2024

Choose a reason for hiding this comment

greptile-apps bot Dec 28, 2024

Choose a reason for hiding this comment

greptile-apps bot Dec 28, 2024

Choose a reason for hiding this comment

greptile-apps bot Dec 28, 2024

Choose a reason for hiding this comment

greptile-apps bot Dec 28, 2024

Choose a reason for hiding this comment

greptile-apps bot Dec 28, 2024

Choose a reason for hiding this comment

greptile-apps bot Dec 28, 2024

Choose a reason for hiding this comment

greptile-apps bot Dec 28, 2024

Choose a reason for hiding this comment

oddgrd commented Dec 31, 2024

oddgrd commented Jan 9, 2025