-
Notifications
You must be signed in to change notification settings - Fork 0
examples #8
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
base: main
Are you sure you want to change the base?
Conversation
Co-Authored-By: hey-ewan <ewanretorokugbe@gmail.com> Co-Authored-By: g4titanx <g4titan1@gmail.com>
WalkthroughThis pull request updates the project’s example configuration and dependencies. The Changes
Sequence Diagram(s)sequenceDiagram
participant MAIN as main()
participant NOTARY as NotaryClient/Service
participant TCP as TCP Connection
participant TLS as TLS Handshake
participant HTTP as HTTP Server
MAIN->>NOTARY: Request notarisation (obtain session ID)
NOTARY-->>MAIN: Return session ID
MAIN->>TCP: Establish TCP connection
TCP->>TLS: Initiate TLS handshake
TLS-->>TCP: Confirm TLS connection
MAIN->>HTTP: Send HTTP request with headers
HTTP-->>MAIN: Return HTTP response (status 200)
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 6
🧹 Nitpick comments (2)
examples/x.com/x_post.rs (2)
13-13: Address the TODO comment about the GraphQL route.The comment indicates that the URL needs to be updated with the correct GraphQL route. Please verify and update the route accordingly.
Would you like me to help identify the correct GraphQL route for x.com's API?
16-16: Consider making the notary host configurable.The notary host is hardcoded to localhost. Consider making this configurable through environment variables for different deployment scenarios.
-const NOTARY_HOST: &str = "127.0.0.1:7047"; +const NOTARY_HOST: &str = env!("NOTARY_HOST", "127.0.0.1:7047");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
examples/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
examples/Cargo.toml(1 hunks)examples/twitter/lenghty_post.rs(0 hunks)examples/x.com/x_post.rs(1 hunks)
💤 Files with no reviewable changes (1)
- examples/twitter/lenghty_post.rs
🔇 Additional comments (1)
examples/Cargo.toml (1)
8-9: LGTM! Example configuration updated to reflect X rebranding.The example name and path have been appropriately updated to reflect the rebranding from Twitter to X.
examples/x.com/x_post.rs
Outdated
| let prover_config = ProverConfig::builder() | ||
| .id(session_id) | ||
| .server_dns(SERVER_DOMAIN) | ||
| .build() | ||
| .unwrap(); | ||
| let prover = Prover::new(prover_config) | ||
| .setup(notary_connection.compat()) | ||
| .await | ||
| .unwrap(); |
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.
🛠️ Refactor suggestion
Improve error handling for prover setup.
The prover setup uses unwrap() which will panic on errors. Consider implementing proper error handling.
- let prover_config = ProverConfig::builder()
- .id(session_id)
- .server_dns(SERVER_DOMAIN)
- .build()
- .unwrap();
- let prover = Prover::new(prover_config)
- .setup(notary_connection.compat())
- .await
- .unwrap();
+ let prover_config = ProverConfig::builder()
+ .id(session_id)
+ .server_dns(SERVER_DOMAIN)
+ .build()
+ .expect("Failed to create prover config");
+ let prover = Prover::new(prover_config)
+ .setup(notary_connection.compat())
+ .await
+ .expect("Failed to setup prover");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let prover_config = ProverConfig::builder() | |
| .id(session_id) | |
| .server_dns(SERVER_DOMAIN) | |
| .build() | |
| .unwrap(); | |
| let prover = Prover::new(prover_config) | |
| .setup(notary_connection.compat()) | |
| .await | |
| .unwrap(); | |
| let prover_config = ProverConfig::builder() | |
| .id(session_id) | |
| .server_dns(SERVER_DOMAIN) | |
| .build() | |
| .expect("Failed to create prover config"); | |
| let prover = Prover::new(prover_config) | |
| .setup(notary_connection.compat()) | |
| .await | |
| .expect("Failed to setup prover"); |
examples/x.com/x_post.rs
Outdated
| let notary_client = NotaryClient::builder().host(NOTARY_HOST).build().unwrap(); | ||
| let notarization_request = NotarizationRequest::builder().build().unwrap(); | ||
|
|
||
| let Accepted { | ||
| io: notary_connection, | ||
| id: session_id, | ||
| .. | ||
| } = notary_client | ||
| .request_notarization(notarization_request) | ||
| .await | ||
| .unwrap(); |
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.
🛠️ Refactor suggestion
Improve error handling for notary client setup.
The notary client setup uses unwrap() which will panic on errors. Consider implementing proper error handling to provide better feedback.
- let notary_client = NotaryClient::builder().host(NOTARY_HOST).build().unwrap();
- let notarization_request = NotarizationRequest::builder().build().unwrap();
+ let notary_client = NotaryClient::builder()
+ .host(NOTARY_HOST)
+ .build()
+ .expect("Failed to create notary client");
+ let notarization_request = NotarizationRequest::builder()
+ .build()
+ .expect("Failed to create notarization request");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let notary_client = NotaryClient::builder().host(NOTARY_HOST).build().unwrap(); | |
| let notarization_request = NotarizationRequest::builder().build().unwrap(); | |
| let Accepted { | |
| io: notary_connection, | |
| id: session_id, | |
| .. | |
| } = notary_client | |
| .request_notarization(notarization_request) | |
| .await | |
| .unwrap(); | |
| let notary_client = NotaryClient::builder() | |
| .host(NOTARY_HOST) | |
| .build() | |
| .expect("Failed to create notary client"); | |
| let notarization_request = NotarizationRequest::builder() | |
| .build() | |
| .expect("Failed to create notarization request"); | |
| let Accepted { | |
| io: notary_connection, | |
| id: session_id, | |
| .. | |
| } = notary_client | |
| .request_notarization(notarization_request) | |
| .await | |
| .unwrap(); |
examples/x.com/x_post.rs
Outdated
| let client_socket = tokio::net::TcpStream::connect((SERVER_DOMAIN, 443)) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| let (tls_connection, prover_fut) = prover.connect(client_socket.compat()).await.unwrap(); | ||
| let tls_connection = TokioIo::new(tls_connection.compat()); | ||
|
|
||
| let prover_ctrl = prover_fut.control(); | ||
|
|
||
| let prover_task = tokio::spawn(prover_fut); | ||
|
|
||
| let (mut request_sender, connection) = hyper::client::conn::http1::handshake(tls_connection) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| tokio::spawn(connection); |
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.
🛠️ Refactor suggestion
Improve error handling for TLS connection setup.
The TLS connection setup uses unwrap() which will panic on errors. Consider implementing proper error handling.
- let client_socket = tokio::net::TcpStream::connect((SERVER_DOMAIN, 443))
- .await
- .unwrap();
-
- let (tls_connection, prover_fut) = prover.connect(client_socket.compat()).await.unwrap();
- let tls_connection = TokioIo::new(tls_connection.compat());
-
- let prover_ctrl = prover_fut.control();
-
- let prover_task = tokio::spawn(prover_fut);
-
- let (mut request_sender, connection) = hyper::client::conn::http1::handshake(tls_connection)
- .await
- .unwrap();
+ let client_socket = tokio::net::TcpStream::connect((SERVER_DOMAIN, 443))
+ .await
+ .expect("Failed to connect to server");
+
+ let (tls_connection, prover_fut) = prover.connect(client_socket.compat())
+ .await
+ .expect("Failed to establish TLS connection");
+ let tls_connection = TokioIo::new(tls_connection.compat());
+
+ let prover_ctrl = prover_fut.control();
+
+ let prover_task = tokio::spawn(prover_fut);
+
+ let (mut request_sender, connection) = hyper::client::conn::http1::handshake(tls_connection)
+ .await
+ .expect("Failed to perform HTTP handshake");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let client_socket = tokio::net::TcpStream::connect((SERVER_DOMAIN, 443)) | |
| .await | |
| .unwrap(); | |
| let (tls_connection, prover_fut) = prover.connect(client_socket.compat()).await.unwrap(); | |
| let tls_connection = TokioIo::new(tls_connection.compat()); | |
| let prover_ctrl = prover_fut.control(); | |
| let prover_task = tokio::spawn(prover_fut); | |
| let (mut request_sender, connection) = hyper::client::conn::http1::handshake(tls_connection) | |
| .await | |
| .unwrap(); | |
| tokio::spawn(connection); | |
| let client_socket = tokio::net::TcpStream::connect((SERVER_DOMAIN, 443)) | |
| .await | |
| .expect("Failed to connect to server"); | |
| let (tls_connection, prover_fut) = prover.connect(client_socket.compat()) | |
| .await | |
| .expect("Failed to establish TLS connection"); | |
| let tls_connection = TokioIo::new(tls_connection.compat()); | |
| let prover_ctrl = prover_fut.control(); | |
| let prover_task = tokio::spawn(prover_fut); | |
| let (mut request_sender, connection) = hyper::client::conn::http1::handshake(tls_connection) | |
| .await | |
| .expect("Failed to perform HTTP handshake"); | |
| tokio::spawn(connection); |
examples/x.com/x_post.rs
Outdated
| // Load secret variables from environment for twitter server connection | ||
| dotenv::dotenv().ok(); | ||
| let post_id = env::var("POST_ID").unwrap(); | ||
| let auth_token = env::var("AUTH_TOKEN").unwrap(); | ||
| let access_token = env::var("ACCESS_TOKEN").unwrap(); | ||
| let csrf_token = env::var("CSRF_TOKEN").unwrap(); | ||
|
|
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.
🛠️ Refactor suggestion
Improve error handling for environment variables.
The current implementation uses unwrap() which will panic if environment variables are missing. Consider implementing proper error handling.
- let post_id = env::var("POST_ID").unwrap();
- let auth_token = env::var("AUTH_TOKEN").unwrap();
- let access_token = env::var("ACCESS_TOKEN").unwrap();
- let csrf_token = env::var("CSRF_TOKEN").unwrap();
+ let post_id = env::var("POST_ID")
+ .expect("POST_ID environment variable not set");
+ let auth_token = env::var("AUTH_TOKEN")
+ .expect("AUTH_TOKEN environment variable not set");
+ let access_token = env::var("ACCESS_TOKEN")
+ .expect("ACCESS_TOKEN environment variable not set");
+ let csrf_token = env::var("CSRF_TOKEN")
+ .expect("CSRF_TOKEN environment variable not set");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Load secret variables from environment for twitter server connection | |
| dotenv::dotenv().ok(); | |
| let post_id = env::var("POST_ID").unwrap(); | |
| let auth_token = env::var("AUTH_TOKEN").unwrap(); | |
| let access_token = env::var("ACCESS_TOKEN").unwrap(); | |
| let csrf_token = env::var("CSRF_TOKEN").unwrap(); | |
| // Load secret variables from environment for twitter server connection | |
| dotenv::dotenv().ok(); | |
| let post_id = env::var("POST_ID") | |
| .expect("POST_ID environment variable not set"); | |
| let auth_token = env::var("AUTH_TOKEN") | |
| .expect("AUTH_TOKEN environment variable not set"); | |
| let access_token = env::var("ACCESS_TOKEN") | |
| .expect("ACCESS_TOKEN environment variable not set"); | |
| let csrf_token = env::var("CSRF_TOKEN") | |
| .expect("CSRF_TOKEN environment variable not set"); |
examples/x.com/x_post.rs
Outdated
| let request = Request::builder() | ||
| .uri(format!("https://{SERVER_DOMAIN}/{ROUTE}/{post_id}.json")) | ||
| .header("Host", SERVER_DOMAIN) | ||
| .header("Accept", "*/*") | ||
| .header("Accept-Encoding", "identity") | ||
| .header("Connection", "close") | ||
| .header("User-Agent", USER_AGENT) | ||
| .header("Authorization", format!("Bearer {access_token}")) | ||
| .header( | ||
| "Cookie", | ||
| format!("auth_token={auth_token}; ct0={csrf_token}"), | ||
| ) | ||
| .header("Authority", SERVER_DOMAIN) | ||
| .header("X-Twitter-Auth-Type", "OAuth2Session") | ||
| .header("x-twitter-active-user", "yes") | ||
| .header("X-Csrf-Token", csrf_token.clone()) | ||
| .body(Empty::<Bytes>::new()) | ||
| .unwrap(); | ||
|
|
||
| debug!("Sending request"); | ||
|
|
||
| prover_ctrl.defer_decryption().await.unwrap(); | ||
|
|
||
| let response = request_sender.send_request(request).await.unwrap(); | ||
|
|
||
| debug!("Sent request"); | ||
|
|
||
| assert!(response.status() == StatusCode::OK, "{}", response.status()); | ||
|
|
||
| debug!("Request OK"); | ||
| } |
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.
🛠️ Refactor suggestion
Improve request handling and add response body processing.
The request handling uses unwrap() and assert! which will panic on errors. Additionally, the response body is not being processed.
- let request = Request::builder()
- .uri(format!("https://{SERVER_DOMAIN}/{ROUTE}/{post_id}.json"))
- .header("Host", SERVER_DOMAIN)
- // ... other headers ...
- .body(Empty::<Bytes>::new())
- .unwrap();
-
- debug!("Sending request");
-
- prover_ctrl.defer_decryption().await.unwrap();
-
- let response = request_sender.send_request(request).await.unwrap();
-
- debug!("Sent request");
-
- assert!(response.status() == StatusCode::OK, "{}", response.status());
-
- debug!("Request OK");
+ let request = Request::builder()
+ .uri(format!("https://{SERVER_DOMAIN}/{ROUTE}/{post_id}.json"))
+ .header("Host", SERVER_DOMAIN)
+ // ... other headers ...
+ .body(Empty::<Bytes>::new())
+ .expect("Failed to build request");
+
+ debug!("Sending request");
+
+ prover_ctrl.defer_decryption()
+ .await
+ .expect("Failed to defer decryption");
+
+ let response = request_sender.send_request(request)
+ .await
+ .expect("Failed to send request");
+
+ debug!("Sent request");
+
+ if response.status() != StatusCode::OK {
+ panic!("Unexpected response status: {}", response.status());
+ }
+
+ debug!("Request OK");
+
+ // Process response body
+ let body_bytes = response.collect()
+ .await
+ .expect("Failed to read response body")
+ .to_bytes();
+ debug!("Response body: {:?}", String::from_utf8_lossy(&body_bytes));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let request = Request::builder() | |
| .uri(format!("https://{SERVER_DOMAIN}/{ROUTE}/{post_id}.json")) | |
| .header("Host", SERVER_DOMAIN) | |
| .header("Accept", "*/*") | |
| .header("Accept-Encoding", "identity") | |
| .header("Connection", "close") | |
| .header("User-Agent", USER_AGENT) | |
| .header("Authorization", format!("Bearer {access_token}")) | |
| .header( | |
| "Cookie", | |
| format!("auth_token={auth_token}; ct0={csrf_token}"), | |
| ) | |
| .header("Authority", SERVER_DOMAIN) | |
| .header("X-Twitter-Auth-Type", "OAuth2Session") | |
| .header("x-twitter-active-user", "yes") | |
| .header("X-Csrf-Token", csrf_token.clone()) | |
| .body(Empty::<Bytes>::new()) | |
| .unwrap(); | |
| debug!("Sending request"); | |
| prover_ctrl.defer_decryption().await.unwrap(); | |
| let response = request_sender.send_request(request).await.unwrap(); | |
| debug!("Sent request"); | |
| assert!(response.status() == StatusCode::OK, "{}", response.status()); | |
| debug!("Request OK"); | |
| } | |
| let request = Request::builder() | |
| .uri(format!("https://{SERVER_DOMAIN}/{ROUTE}/{post_id}.json")) | |
| .header("Host", SERVER_DOMAIN) | |
| .header("Accept", "*/*") | |
| .header("Accept-Encoding", "identity") | |
| .header("Connection", "close") | |
| .header("User-Agent", USER_AGENT) | |
| .header("Authorization", format!("Bearer {access_token}")) | |
| .header( | |
| "Cookie", | |
| format!("auth_token={auth_token}; ct0={csrf_token}"), | |
| ) | |
| .header("Authority", SERVER_DOMAIN) | |
| .header("X-Twitter-Auth-Type", "OAuth2Session") | |
| .header("x-twitter-active-user", "yes") | |
| .header("X-Csrf-Token", csrf_token.clone()) | |
| .body(Empty::<Bytes>::new()) | |
| .expect("Failed to build request"); | |
| debug!("Sending request"); | |
| prover_ctrl.defer_decryption() | |
| .await | |
| .expect("Failed to defer decryption"); | |
| let response = request_sender.send_request(request) | |
| .await | |
| .expect("Failed to send request"); | |
| debug!("Sent request"); | |
| if response.status() != StatusCode::OK { | |
| panic!("Unexpected response status: {}", response.status()); | |
| } | |
| debug!("Request OK"); | |
| // Process response body | |
| let body_bytes = response.collect() | |
| .await | |
| .expect("Failed to read response body") | |
| .to_bytes(); | |
| debug!("Response body: {:?}", String::from_utf8_lossy(&body_bytes)); | |
| } |
| tlsn-core = { git = "https://github.com/tlsnotary/tlsn.git", tag = "v0.1.0-alpha.6", package = "tlsn-core" } | ||
| notary-client = { git = "https://github.com/tlsnotary/tlsn.git", tag = "v0.1.0-alpha.6", package = "notary-client" } | ||
| tlsn-prover = { git = "https://github.com/tlsnotary/tlsn.git", tag = "v0.1.0-alpha.6", package = "tlsn-prover" } |
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.
💡 Verification agent
🧩 Analysis chain
Verify TLS notary dependency versions.
The TLS notary dependencies are using git tags. Please verify that these are the latest stable versions.
🏁 Script executed:
#!/bin/bash
# Check latest tags for TLS notary dependencies
gh api repos/tlsnotary/tlsn/tags --jq '.[].name' | head -n 5Length of output: 137
TLS notary dependency version update required
In examples/Cargo.toml, the dependencies for tlsn-core, notary-client, and tlsn-prover are currently pinned to v0.1.0-alpha.6. However, the latest available tag in the repository is v0.1.0-alpha.7. Please verify whether this older tag was intentional or if the dependencies should be updated to the more recent v0.1.0-alpha.7.
Docstrings generation was requested by @hey-ewan. * #8 (comment) The following files were modified: * `examples/x.com/x_post.rs`
|
Note Generated docstrings for this pull request at #9 |
📝 Add docstrings to `examples`
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
examples/x.com/x_post.rs (2)
16-16: Consider making the notary host configurable.
It might be beneficial to load the host from an environment variable or configuration file for flexibility.
81-81: Await or handle the spawned task
tokio::spawn(prover_fut)returns aJoinHandle, but it is never awaited or used. Consider awaiting or handling potential errors.let prover_task = tokio::spawn(prover_fut); + // e.g. consider awaiting: + if let Err(e) = prover_task.await { + eprintln!("Prover task failed: {:?}", e); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/x.com/x_post.rs(1 hunks)
🔇 Additional comments (8)
examples/x.com/x_post.rs (8)
1-14: No major concerns for these imports and constants.
They look appropriate for the functionality at hand.
13-13: Confirm correctness of the API route.
The inline comment suggests a to-do to figure out the correct GraphQL route. Ensure that this endpoint is final or update it accordingly.
18-18: Usage of #[tokio::main] attribute
Good use of the attribute for asynchronous entry. No issues noted.
45-48: Improve error handling for environment variables
Usingunwrap()will cause a panic if variables are missing or invalid.
50-61: Improve error handling for notary client setup
These.unwrap()calls may cause a panic.
62-70: Improve error handling for prover setup
These builder operations that end with.unwrap()will panic on failure.
72-87: Improve error handling for TLS connection setup
All.unwrap()calls in this section may lead to panics if any step fails.
89-119: Improve request handling and add response body processing
These.unwrap()andassert!calls can cause the application to panic on non-OK status or unexpected inputs.
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
examples/src/x.rs (2)
71-80: Use structured error handling or logging for TCP connection failures.Wrapping your
TcpStream::connectandprover.connectcalls in robust error handling or logging can help diagnose connectivity issues in real-world conditions.
109-115: Refine assertion for production environments.Relying on
assert!for HTTP status checks will panic if the response is not 200 OK. In production, consider returning or handling errors more gracefully, logging possible failure causes, etc.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/src/lib.rs(1 hunks)examples/src/x.rs(1 hunks)examples/x.com/x_post.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/x.com/x_post.rs
🔇 Additional comments (5)
examples/src/lib.rs (1)
1-1: No concerns regarding the new module declaration.Declaring
mod x;cleanly organises code into a new module. Everything seems in order, and no further action is required here.examples/src/x.rs (4)
1-11: Overall import usage appears correct and organised.The chosen crates—
hyper,tokio, and others—seem suitable for an asynchronous HTTP workflow. This setup is coherent with the objective of making notarised requests, and the line-up of dependencies is comprehensive. No immediate changes needed.
12-13: Confirm whether the route is correct.There is a comment suggesting the current route might be a placeholder (
"i/api/1.1/"). Consider aligning it with the correct GraphQL-based or updated v2/v3 route if needed.
18-38: Good use of doc comments.The comment block clearly details usage, prerequisites, and examples. This thorough explanation is beneficial for future maintainers and users, reflecting best practices for Rust documentation.
88-107: Validate or mask sensitive tokens to avoid accidental logging.You're sending
auth_tokenandcsrf_tokenin request headers. If logs are set to a high verbosity level, these might be exposed. Consider redacting or minimising logs for sensitive fields.
[security]
| /// | ||
| /// run(); | ||
| /// ``` | ||
| pub async fn notarize() { |
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.
🛠️ Refactor suggestion
Consider returning a Result instead of panicking on errors.
pub async fn notarize() uses several unwrap() calls that will panic if an operation fails (e.g., environment variables are missing). In production scenarios, prefer error handling to avoid abrupt termination.
- pub async fn notarize() {
+ pub async fn notarize() -> Result<(), Box<dyn std::error::Error>> {
...
let post_id = env::var("POST_ID")?;
...
// Propagate errors up instead of panicking
Ok(())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub async fn notarize() { | |
| pub async fn notarize() -> Result<(), Box<dyn std::error::Error>> { | |
| // ... | |
| let post_id = env::var("POST_ID")?; | |
| // ... | |
| // Propagate errors up instead of panicking | |
| Ok(()) | |
| } |
set up examples to show off The Forge's functionalities
Summary by CodeRabbit
New Features
Chores