Skip to content

feat: Pluggable transport interface + NetHTTP default transport. - #357

Open
sichanyoo wants to merge 3 commits into
mainfrom
pluggable-transport
Open

feat: Pluggable transport interface + NetHTTP default transport.#357
sichanyoo wants to merge 3 commits into
mainfrom
pluggable-transport

Conversation

@sichanyoo

Copy link
Copy Markdown
Contributor

Issue #, if available:
5196

Description of changes:

AI-generated summary of changes below

============================================================================================
smithy-ruby -- Pluggable Transport changes: what & why
Commit b5f2f092  "Pluggable transport interface + NetHTTP default transport."
Generated 2026-08-24 15:59:35 -0700
============================================================================================

 1. [MODIFIED] gems/smithy-client/lib/smithy-client.rb  (+3 / -1)
--------------------------------------------------------------------------------------------
    Adjusts the gem require order for the new transport stack: drops the requires for the
    deleted net_http/handler and the old transport.rb contract file, and adds net_http/stream,
    net_http/transport, and the generic send_handler (loaded after net_http/connection_pool so
    dependencies resolve in order).

 2. [MODIFIED] gems/smithy-client/lib/smithy-client/errors.rb  (+7 / -0)
--------------------------------------------------------------------------------------------
    Introduces Smithy::Client::NotSupportedError (it used to live in the now-deleted transport
    contract file). HTTP/1.1 streams raise it from #write / #close_write because a request
    body cannot be written after the request is transmitted -- that is HTTP/2 event-stream
    territory.

 3. [DELETED ] gems/smithy-client/lib/smithy-client/net_http/handler.rb  (+0 / -157)
--------------------------------------------------------------------------------------------
    Deletes the old push/callback Net::HTTP handler. Its job is replaced by the new split: a
    NetHTTP::Transport returning a pull-based Stream, plus a transport-agnostic SendHandler.
    This is the heart of the pluggable-transport refactor.

 4. [NEW     ] gems/smithy-client/lib/smithy-client/net_http/stream.rb  (+323 / -0)
--------------------------------------------------------------------------------------------
    New. The fiber-backed HTTP/1.1 stream implementing the pull contract: response_headers,
    each_chunk, write / close_write (raise NotSupportedError), and abort. A Fiber keeps
    Net::HTTP's block-scoped read_body context alive across the separate response_headers and
    each_chunk calls (that block also holds the socket's buffered bytes, including any body
    bytes over-read while parsing headers). Robustness added during review: (1) Guaranteed
    teardown -- if the consumer raises or the body isn't fully read, the connection is
    finished, not leaked. (2) Truncation detection -- Net::HTTP defaults ignore_eof: true and
    silently returns short Content-Length bodies, so the byte count is verified inside the
    fiber (right after read_body, still inside the pool session block) and a truncated,
    peer-closed socket is finished instead of returned to the pool. (3) Thread safety -- a
    mutex guards the @done/@aborted/@error/@session transitions so a cross-thread abort is
    observed and cannot race normal completion, and the fiber reconciles an abort that raced
    ahead of session checkout so the session is never orphaned. Note: the pull-based model
    holds on all runtimes, but the "no background OS thread" property is CRuby-only
    (JRuby/TruffleRuby back fibers with a thread).

 5. [NEW     ] gems/smithy-client/lib/smithy-client/net_http/transport.rb  (+154 / -0)
--------------------------------------------------------------------------------------------
    New. NetHTTP::Transport is the default HTTP/1.1 transport and the documented swap point
    (the :transport option). It exposes a single transmit(request) -> Stream, maps the
    transport-agnostic client options (connect/read timeouts, TLS verify + trust store, proxy,
    wire trace) onto the Net::HTTP ConnectionPool option names, and accepts Net::HTTP-specific
    knobs (continue/keep-alive/write/ssl timeouts, client cert/key) as constructor-only
    kwargs. The class doc carries a full V3-option equivalence table. close was intentionally
    omitted: with the global H1 pool it is not meaningful, and V3 has no H1 equivalent.

 6. [MODIFIED] gems/smithy-client/lib/smithy-client/net_http/connection_pool.rb  (+27 / -4)
--------------------------------------------------------------------------------------------
    Hardens TLS setup. configure_ssl now sets http.verify_mode explicitly so ssl_verify_peer:
    false is actually honored (previously it was silently ignored and always verified), and
    warns through the logger when verification is disabled. The old cert method is split into
    configure_client_cert (client cert/key for mutual TLS -- always applied, since it is
    independent of server verification) and configure_ca_trust (CA bundle/path/store --
    applied only when peer verification is on).

 7. [DELETED ] gems/smithy-client/lib/smithy-client/plugins/net_http.rb  (+0 / -163)
--------------------------------------------------------------------------------------------
    Deletes the old Net::HTTP plugin. Its options are re-homed: transport-agnostic ones move
    to the new generic Transport plugin; Net::HTTP-specific ones become constructor kwargs on
    NetHTTP::Transport (no longer client config).

 8. [NEW     ] gems/smithy-client/lib/smithy-client/plugins/transport.rb  (+172 / -0)
--------------------------------------------------------------------------------------------
    New. The generic, transport-neutral plugin. It defines the transport-agnostic client
    options (connect_timeout, read_timeout, ssl_verify_peer, ssl_ca_bundle/directory/store,
    http_proxy, http_wire_trace), resolves the :transport default (a NetHTTP::Transport built
    from those options, forwarded via a single TRANSPORT_OPTIONS list to avoid drift), and
    registers the SendHandler at the :send step. It validates a customer-supplied :transport
    in before_initialize (must respond to #transmit) -- chosen over after_initialize so the
    default transport isn't eagerly built just to validate it.

 9. [NEW     ] gems/smithy-client/lib/smithy-client/send_handler.rb  (+73 / -0)
--------------------------------------------------------------------------------------------
    New. The transport-agnostic :send handler. It calls transport.transmit(request), then
    bridges the pulled Stream bytes onto the push-based Http::Response (signal headers ->
    each_chunk -> signal_data -> signal_done). Blocking is a handler-stack decision, not a
    transport one: it resolves the response immediately for plain request/response but returns
    early for a bidirectional duplex_stream (owned by the future event-stream layer). An
    ensure guarantees the stream is aborted on any early/error exit so the connection is
    released.

10. [NEW     ] gems/smithy-client/spec/smithy-client/net_http/stream_spec.rb  (+100 / -0)
--------------------------------------------------------------------------------------------
    New. Unit coverage for the Stream: header/status reads, body chunk streaming, empty body,
    invalid-verb ArgumentError without networking, networking-error wrapping, write /
    close_write raising, and abort.

11. [NEW     ] gems/smithy-client/spec/smithy-client/net_http/transport_spec.rb  (+110 / -0)
--------------------------------------------------------------------------------------------
    New. Coverage for NetHTTP::Transport: transmit returns a Stream, the transport-agnostic ->
    Net::HTTP option mapping, and default wiring through the plugin.

12. [NEW     ] gems/smithy-client/spec/smithy-client/plugins/transport_spec.rb  (+56 / -0)
--------------------------------------------------------------------------------------------
    New. Coverage for the Transport plugin: tier-a options are defined, a default NetHTTP
    transport resolves, a customer-supplied transport is used as-is (and one lacking #transmit
    is rejected), option forwarding, and SendHandler registration.

13. [NEW     ] gems/smithy-client/spec/smithy-client/send_handler_spec.rb  (+126 / -0)
--------------------------------------------------------------------------------------------
    New. Coverage for the SendHandler: status/header/body population, invalid-verb and
    networking/OpenSSL error signaling, the Content-Length truncation raise, HEAD (no byte
    verification), and that a duplex_stream stores the stream without resolving the response.

14. [DELETED ] gems/smithy-client/spec/smithy-client/net_http/handler_spec.rb  (+0 / -174)
--------------------------------------------------------------------------------------------
    Deleted along with the old handler it tested.

15. [DELETED ] gems/smithy-client/spec/smithy-client/plugins/net_http_spec.rb  (+0 / -37)
--------------------------------------------------------------------------------------------
    Deleted along with the old Net::HTTP plugin it tested.

16. [MODIFIED] gems/smithy/lib/smithy/welds/default_plugins.rb  (+2 / -2)
--------------------------------------------------------------------------------------------
    Codegen weld change. Removes the old NetHTTP plugin from the default set and adds the new
    generic Transport plugin, positioned before StubResponses (both register a :send handler
    and the last one added wins, so stubbing must be able to override the real send handler).

17. [MODIFIED] projections/weather/lib/weather/client.rb  (+9 / -0)
--------------------------------------------------------------------------------------------
    Regenerated sample client showing the weld effect: the Transport plugin and :transport
    option are added (before StubResponses), replacing the old NetHTTP wiring.

============================================================================================
Totals: 17 files,  +1162 insertions,  -538 deletions
============================================================================================

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@sichanyoo
sichanyoo requested a review from a team as a code owner August 24, 2026 23:01
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