Skip to content

Conversation

Mossaka
Copy link
Collaborator

@Mossaka Mossaka commented Oct 4, 2025

Signed-off-by: Jiaxiao Zhou [email protected]

…enforce URL scheme & redirect loop detection, improve decoding & error reporting

Signed-off-by: Jiaxiao Zhou <[email protected]>
@Mossaka Mossaka requested a review from Copilot October 4, 2025 00:04
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR significantly enhances the fetch-rs tool by adding comprehensive error handling, environment-configurable options, URL scheme validation, redirect management, and improved content decoding. The changes transform the simple fetch tool into a robust HTTP client with production-ready features.

  • Adds environment-configurable options for limits (redirects, body size), timeout, and user agent
  • Implements URL scheme validation, redirect loop detection, and comprehensive error handling
  • Introduces content decoding support for gzip, deflate, and brotli compression with detailed response metadata

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.

File Description
examples/fetch-rs/src/lib.rs Complete rewrite adding structured error handling, redirect management, content decoding, and environment configuration
examples/fetch-rs/Cargo.toml Adds new dependencies for compression, URL parsing, MIME type handling, and removes scraper

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +521 to +522
if let Some(limit) = options.timeout_ms() {
if metrics.elapsed_ms > limit as u128 {
Copy link

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

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

The timeout check occurs after the request completes and body is processed, making it ineffective. The timeout should be enforced during the HTTP request itself, not after completion.

Copilot uses AI. Check for mistakes.

.map(|part| part.trim().to_ascii_lowercase())
.filter(|part| !part.is_empty())
.collect();

Copy link

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

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

The reason for reversing the encoding order should be documented. HTTP content-encoding headers list encodings in the order they were applied, so they must be decoded in reverse order.

Suggested change
// HTTP content-encoding headers list encodings in the order they were applied,
// so they must be decoded in reverse order.

Copilot uses AI. Check for mistakes.

Comment on lines +742 to +748
fn clip_bytes(data: &[u8], limit: usize) -> (Vec<u8>, bool) {
if data.len() > limit {
(data[..limit].to_vec(), true)
} else {
(data.to_vec(), false)
}
}
Copy link

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

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

The function always clones the data even when no clipping is needed. Consider returning a slice reference when not truncated to avoid unnecessary allocation.

Copilot uses AI. Check for mistakes.

Comment on lines +785 to +786
.skip_while(|b| b.is_ascii_whitespace())
.next()
Copy link

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

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

The iterator chain could be simplified using find() instead of skip_while().next() for better readability and potentially better performance.

Suggested change
.skip_while(|b| b.is_ascii_whitespace())
.next()
.find(|b| !b.is_ascii_whitespace())

Copilot uses AI. Check for mistakes.

@Mossaka
Copy link
Collaborator Author

Mossaka commented Oct 9, 2025

@copilot open a new pull request to apply changes based on the comments in this thread and fix this test failure:

Error: Timeout waiting for tool list change notification

Caused by:
    deadline has elapsed

Stack backtrace:
   0: <E as anyhow::context::ext::StdError>::ext_context
test test_tool_list_notification ... FAILED
   1: anyhow::context::<impl anyhow::Context<T,E> for core::result::Result<T,E>>::context
   2: transport_integration_test::test_tool_list_notification::{{closure}}
   3: <core::pin::Pin<P> as core::future::future::Future>::poll
   4: <core::pin::Pin<P> as core::future::future::Future>::poll
   5: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::{{closure}}::{{closure}}
   6: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::{{closure}}
   7: tokio::runtime::scheduler::current_thread::Context::enter
   8: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}
   9: tokio::runtime::scheduler::current_thread::CoreGuard::enter::{{closure}}
  10: tokio::runtime::context::scoped::Scoped<T>::set
  11: tokio::runtime::context::set_scheduler::{{closure}}
  12: std::thread::local::LocalKey<T>::try_with
  13: std::thread::local::LocalKey<T>::with
  14: tokio::runtime::context::set_scheduler
  15: tokio::runtime::scheduler::current_thread::CoreGuard::enter
  16: tokio::runtime::scheduler::current_thread::CoreGuard::block_on
  17: tokio::runtime::scheduler::current_thread::CurrentThread::block_on::{{closure}}
  18: tokio::runtime::context::runtime::enter_runtime
  19: tokio::runtime::scheduler::current_thread::CurrentThread::block_on
  20: tokio::runtime::runtime::Runtime::block_on_inner
  21: tokio::runtime::runtime::Runtime::block_on
  22: transport_integration_test::test_tool_list_notification
  23: transport_integration_test::test_tool_list_notification::{{closure}}
  24: core::ops::function::FnOnce::call_once
  25: test::__rust_begin_short_backtrace
  26: test::run_test::{{closure}}
  27: std::sys::backtrace::__rust_begin_short_backtrace
  28: core::ops::function::FnOnce::call_once{{vtable.shim}}
  29: std::sys::pal::unix::thread::Thread::new::thread_start
  30: <unknown>
  31: <unknown>
2025-10-04T00:25:12.836547Z  INFO load_component{uri="file:///home/runner/work/wassette/wassette/examples/fetch-rs/target/wasm32-wasip2/release/fetch2.wasm"}: wassette: Saved precompiled component component_id=fetch2
2025-10-04T00:25:12.838490Z  INFO load_component{uri="file:///home/runner/work/wassette/wassette/examples/fetch-rs/target/wasm32-wasip2/release/fetch2.wasm"}: wassette: Saved component metadata component_id=fetch2
2025-10-04T00:25:12.839200Z  INFO load_component{uri="file:///home/runner/work/wassette/wassette/examples/fetch-rs/target/wasm32-wasip2/release/fetch2.wasm"}: wassette: Successfully loaded component component_id=fetch2 status=New tools=["fetch"]
test test_fetch_component_workflow ... ok

failures:

failures:
    test_tool_list_notification

@Mossaka
Copy link
Collaborator Author

Mossaka commented Oct 9, 2025

@copilot open a new pull request to apply changes based on the comments in this thread

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.

1 participant