-
Notifications
You must be signed in to change notification settings - Fork 19
feat: Add HttpTransport trait abstraction #136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
keelerm84
wants to merge
1
commit into
mk/sdk-1741/bump-hyper
Choose a base branch
from
mk/sdk-1742/2-create-transport-abstraction
base: mk/sdk-1741/bump-hyper
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume this is basically the same as the event source one?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| //! HTTP transport abstraction for LaunchDarkly SDK | ||
| //! | ||
| //! This module defines the [`HttpTransport`] trait which allows users to plug in | ||
| //! their own HTTP client implementation (hyper, reqwest, or custom). | ||
|
|
||
| use bytes::Bytes; | ||
| use futures::Stream; | ||
| use std::error::Error as StdError; | ||
| use std::fmt; | ||
| use std::future::Future; | ||
| use std::pin::Pin; | ||
|
|
||
| // Re-export http crate types for convenience | ||
| pub use http::{Request, Response}; | ||
|
|
||
| /// A pinned, boxed stream of bytes returned by HTTP transports. | ||
| /// | ||
| /// This represents the streaming response body from an HTTP request. | ||
| pub type ByteStream = Pin<Box<dyn Stream<Item = Result<Bytes, TransportError>> + Send>>; | ||
|
|
||
| /// A pinned, boxed future for an HTTP response. | ||
| /// | ||
| /// This represents the future returned by [`HttpTransport::request`]. | ||
| pub type ResponseFuture = | ||
| Pin<Box<dyn Future<Output = Result<Response<ByteStream>, TransportError>> + Send>>; | ||
|
|
||
| /// Error type for HTTP transport operations. | ||
| /// | ||
| /// This wraps transport-specific errors (network failures, timeouts, etc.) in a | ||
| /// common error type that the SDK can handle uniformly. | ||
| #[derive(Debug)] | ||
| pub struct TransportError { | ||
| inner: Box<dyn StdError + Send + Sync + 'static>, | ||
| } | ||
|
|
||
| impl TransportError { | ||
| /// Create a new transport error from any error type. | ||
| pub fn new(err: impl StdError + Send + Sync + 'static) -> Self { | ||
| Self { | ||
| inner: Box::new(err), | ||
| } | ||
| } | ||
|
|
||
| /// Get a reference to the inner error. | ||
| pub fn inner(&self) -> &(dyn StdError + Send + Sync + 'static) { | ||
| &*self.inner | ||
| } | ||
| } | ||
|
|
||
| impl fmt::Display for TransportError { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| write!(f, "transport error: {}", self.inner) | ||
| } | ||
| } | ||
|
|
||
| impl StdError for TransportError { | ||
| fn source(&self) -> Option<&(dyn StdError + 'static)> { | ||
| Some(&*self.inner) | ||
| } | ||
| } | ||
|
|
||
| /// Trait for pluggable HTTP transport implementations. | ||
| /// | ||
| /// Implement this trait to provide HTTP request/response functionality for the | ||
| /// SDK. The transport is responsible for: | ||
| /// - Establishing HTTP connections (with TLS if needed) | ||
| /// - Sending HTTP requests | ||
| /// - Returning streaming HTTP responses | ||
| /// - Handling timeouts (if desired) | ||
| /// | ||
| /// The SDK normally uses [`crate::HyperTransport`] as the default implementation, | ||
| /// but you can provide your own implementation for custom requirements such as: | ||
| /// - Using a different HTTP client library (reqwest, custom, etc.) | ||
| /// - Adding request/response logging or metrics | ||
| /// - Implementing custom retry logic | ||
| /// - Using a proxy or custom TLS configuration | ||
| /// | ||
| /// # Example | ||
| /// | ||
| /// ```no_run | ||
| /// use launchdarkly_server_sdk::{HttpTransport, ResponseFuture, TransportError}; | ||
| /// use bytes::Bytes; | ||
| /// use http::{Request, Response}; | ||
| /// | ||
| /// #[derive(Clone)] | ||
| /// struct LoggingTransport<T: HttpTransport> { | ||
| /// inner: T, | ||
| /// } | ||
| /// | ||
| /// impl<T: HttpTransport> HttpTransport for LoggingTransport<T> { | ||
| /// fn request(&self, request: Request<Bytes>) -> ResponseFuture { | ||
| /// println!("Making request to: {}", request.uri()); | ||
| /// self.inner.request(request) | ||
| /// } | ||
| /// } | ||
| /// ``` | ||
| pub trait HttpTransport: Clone + Send + Sync + 'static { | ||
| /// Execute an HTTP request and return a streaming response. | ||
| /// | ||
| /// # Arguments | ||
| /// | ||
| /// * `request` - The HTTP request to execute. The body type is `Bytes` | ||
| /// to support both binary content and empty bodies. Use `Bytes::new()` | ||
| /// for requests with no body (e.g., GET requests). | ||
| /// | ||
| /// # Returns | ||
| /// | ||
| /// A future that resolves to an HTTP response with a streaming body, or a | ||
| /// transport error if the request fails. | ||
| /// | ||
| /// The response includes: | ||
| /// - Status code | ||
| /// - Response headers | ||
| /// - A stream of body bytes | ||
| /// | ||
| /// # Notes | ||
| /// | ||
| /// - The transport should NOT follow redirects - the SDK handles this when needed | ||
| /// - The transport should NOT retry requests - the SDK handles this | ||
| /// - The transport MAY implement timeouts as desired | ||
| fn request(&self, request: Request<Bytes>) -> ResponseFuture; | ||
| } |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is changing this feature name going to be a problem compatibility wise?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or was this a new feature to the branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is getting merged into a separate branch that will become 3.x once the rest of the stuff is done. So I expect to break backwards compatibility with this.