Skip to content

Conversation

@majiayu000
Copy link
Contributor

Summary

  • Add configurable timeout parameter to HTTP request tool
  • Add timeout field to API block UI (default: 120000ms, max: 600000ms)
  • Improved timeout error message with actionable guidance

Test plan

  • All HTTP request tests pass (23 tests)
  • Manual testing with slow API endpoints

Fixes #2242

@vercel
Copy link

vercel bot commented Dec 22, 2025

@majiayu000 is attempting to deploy a commit to the Sim Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 22, 2025

Greptile Summary

Added configurable timeout support to API block requests, solving issue #2242 where API blocks were timing out on slow endpoints. Users can now set custom timeouts up to 10 minutes (default: 2 minutes).

Key Changes

  • Added timeout parameter to HTTP request tool with 120s default and 600s max
  • Added timeout UI field to API block configuration
  • Implemented timeout parsing for both internal and proxy request handlers using AbortSignal.timeout()
  • Added user-friendly timeout error messages with actionable guidance
  • Comprehensive test coverage (23 passing tests)

Minor Issues

  • Timeout parsing logic duplicated in two functions (line 653 and 898 in apps/sim/tools/index.ts) - consider extracting to shared helper
  • Network policy change unrelated to timeout feature but appears safe

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk - well-tested feature addition that solves a real user problem
  • Score reflects solid implementation with comprehensive tests (23 passing), clear error handling, and proper validation. Minor code duplication doesn't affect functionality but reduces maintainability slightly. The feature directly addresses the reported issue with configurable timeouts.
  • Pay attention to apps/sim/tools/index.ts which contains duplicated timeout parsing logic that should be refactored for better maintainability

Important Files Changed

Filename Overview
apps/sim/tools/index.ts Added timeout parsing logic and signal handling for both internal and proxy requests. Code duplication: timeout constants defined twice in same file.
apps/sim/blocks/blocks/api.ts Added timeout UI field to API block configuration. Clean addition with proper description and validation bounds.
apps/sim/tools/http/request.ts Added timeout parameter to HTTP request tool with proper defaults and documentation. Well-implemented.

Sequence Diagram

sequenceDiagram
    participant User
    participant APIBlock
    participant HTTPRequest
    participant InternalHandler
    participant ProxyHandler
    participant ExternalAPI

    User->>APIBlock: Configure timeout (ms)
    APIBlock->>HTTPRequest: Execute with params.timeout
    
    alt Internal Route
        HTTPRequest->>InternalHandler: handleInternalRequest()
        InternalHandler->>InternalHandler: Parse timeout (default: 120s, max: 600s)
        InternalHandler->>InternalHandler: Create AbortSignal.timeout(timeoutMs)
        InternalHandler->>ExternalAPI: fetch(url, {signal})
        
        alt Request completes in time
            ExternalAPI-->>InternalHandler: Response
            InternalHandler-->>HTTPRequest: Success
        else Request times out
            ExternalAPI-->>InternalHandler: TimeoutError
            InternalHandler->>InternalHandler: Log timeout error
            InternalHandler-->>HTTPRequest: "Request timed out after Xms"
        end
    else Proxy Route
        HTTPRequest->>ProxyHandler: handleProxyRequest()
        ProxyHandler->>ProxyHandler: Parse timeout (default: 120s, max: 600s)
        ProxyHandler->>ProxyHandler: Create AbortSignal.timeout(timeoutMs)
        ProxyHandler->>ExternalAPI: fetch(proxyUrl, {signal})
        
        alt Request completes in time
            ExternalAPI-->>ProxyHandler: Response
            ProxyHandler-->>HTTPRequest: Success
        else Request times out
            ExternalAPI-->>ProxyHandler: TimeoutError
            ProxyHandler->>ProxyHandler: Log timeout error
            ProxyHandler-->>HTTPRequest: "Request timed out after Xms"
        end
    end
    
    HTTPRequest-->>APIBlock: Result
    APIBlock-->>User: Display result or timeout error
Loading

@majiayu000
Copy link
Contributor Author

@ARNOLDAJEE I've identified and fixed the issue!

Root Cause: The timeout was only implemented in handleInternalRequest, but external API requests go through handleProxyRequest which was missing the timeout logic. This caused the proxy fetch to use the default timeout instead of your configured 600000ms value.

Fix: Added the same timeout logic to handleProxyRequest:

  • Parse params.timeout parameter (supports both number and string)
  • Add AbortSignal.timeout(timeoutMs) to the proxy fetch call
  • Add user-friendly timeout error message

The fix has been pushed. Could you please test again after this PR is merged?

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. apps/sim/tools/index.ts, line 653-663 (link)

    style: timeout parsing logic duplicated in handleInternalRequest (lines 653-663) and handleProxyRequest (lines 898-908)

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

6 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Lutherwaves and others added 6 commits December 27, 2025 16:13
…dioai#2481)

The realtime service network policy was missing the custom egress rules section
that allows configuration of additional egress rules via values.yaml. This caused
the realtime pods to be unable to connect to external databases (e.g., PostgreSQL
on port 5432) when using external database configurations.

The app network policy already had this section, but the realtime network policy
was missing it, creating an inconsistency and preventing the realtime service
from accessing external databases configured via networkPolicy.egress values.

This fix adds the same custom egress rules template section to the realtime
network policy, matching the app network policy behavior and allowing users to
configure database connectivity via values.yaml.
Adds timeout parameter to HTTP request tool and API block:
- Default timeout: 120000ms (2 minutes)
- Max timeout: 600000ms (10 minutes)
- User-friendly error message on timeout

This allows users to configure longer timeouts for slow API endpoints.

Fixes simstudioai#2242
The previous implementation only added timeout to handleInternalRequest,
but external API requests go through handleProxyRequest which was missing
the timeout logic. This caused the proxy fetch to use default timeout
instead of the user-configured value.

- Add timeout parsing logic to handleProxyRequest
- Add AbortSignal.timeout to proxy fetch call
- Add user-friendly timeout error message

Fixes the issue reported by @ARNOLDAJEE where 600000ms timeout setting
was not taking effect for external API requests.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive unit tests to verify:
- Numeric timeout parsing
- String timeout parsing
- MAX_TIMEOUT_MS cap (600000ms)
- DEFAULT_TIMEOUT_MS fallback (120000ms)
- Invalid value handling
- AbortSignal.timeout integration
- Timeout error identification

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Extract duplicated timeout parsing logic from handleInternalRequest and
handleProxyRequest into a shared parseTimeout() helper function:
- Define DEFAULT_TIMEOUT_MS and MAX_TIMEOUT_MS as module-level constants
- Add parseTimeout() function with proper TSDoc documentation
- Replace inline timeout parsing in both functions

This addresses the code duplication noted in the Greptile review.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@majiayu000 majiayu000 force-pushed the fix/api-block-timeout branch from 4d89f65 to 59177c6 Compare December 27, 2025 08:15
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.

3 participants