Copilot commented on code in PR #4000:
URL: https://github.com/apache/thrift/pull/4000#discussion_r4200483990
##########
lib/netstd/Thrift/Transport/Layered/TBufferedTransport.cs:
##########
@@ -67,6 +67,44 @@ public TTransport UnderlyingTransport
}
}
+ /// <summary>
+ /// Gets the maximum duration of a complete operation delegated to the
underlying transport.
+ /// </summary>
+ public TimeSpan PerCallTimeout
+ {
+ get
+ {
+ if (!(InnerTransport is ITPerCallTransportProvider provider))
+ {
+ throw new System.NotSupportedException("The underlying
transport does not support per-call transports.");
+ }
+ return provider.PerCallTimeout;
+ }
+ }
+
+ /// <summary>
+ /// Gets whether the underlying transport supports per-call transports.
+ /// </summary>
+ public bool SupportsPerCallTransport =>
+ InnerTransport is ITPerCallTransportProvider provider &&
provider.SupportsPerCallTransport;
+
+ /// <summary>
+ /// Creates a buffered transport around a new per-call underlying
transport.
+ /// </summary>
+ /// <param name="cancellationToken">Token used to cancel underlying
transport creation.</param>
+ /// <returns>A new buffered transport owned by the caller.</returns>
+ /// <exception cref="System.NotSupportedException">The underlying
transport does not support per-call transports.</exception>
+ public async Task<TTransport>
CreatePerCallTransportAsync(CancellationToken cancellationToken)
+ {
+ if (!(InnerTransport is ITPerCallTransportProvider provider) ||
!provider.SupportsPerCallTransport)
+ {
+ throw new System.NotSupportedException("The underlying
transport does not support per-call transports.");
+ }
+
+ var transport = await
provider.CreatePerCallTransportAsync(cancellationToken);
+ return new TBufferedTransport(transport, DesiredBufferSize);
Review Comment:
If constructing either memory buffer fails after the provider has created
the call transport, that transport (and an HTTP-client lease beneath it) is
leaked. `TFramedTransport` already protects this same ownership transfer; wrap
buffered-wrapper construction in equivalent exception-safe cleanup.
##########
lib/netstd/Makefile.am:
##########
@@ -74,6 +74,7 @@ dist-hook:
EXTRA_DIST = \
README.md \
+ per-call-transport.md \
Review Comment:
The new `openspec` configuration and specification documents are not
included in `EXTRA_DIST`, so source distributions omit tracked artifacts added
by this PR. Add the directory here; this also follows the new-document
packaging rule in `AGENTS.md:134-137`.
##########
lib/netstd/Thrift/Transport/Layered/TFramedTransport.cs:
##########
@@ -53,6 +53,52 @@ public TFramedTransport(TTransport transport)
InitWriteBuffer();
}
+ /// <summary>
+ /// Gets the maximum duration of a complete operation delegated to the
underlying transport.
+ /// </summary>
+ public TimeSpan PerCallTimeout
+ {
+ get
+ {
+ if (!(InnerTransport is ITPerCallTransportProvider provider))
+ {
+ throw new NotSupportedException("The underlying transport
does not support per-call transports.");
+ }
+ return provider.PerCallTimeout;
+ }
+ }
+
+ /// <summary>
+ /// Gets whether the underlying transport supports per-call transports.
+ /// </summary>
+ public bool SupportsPerCallTransport =>
+ InnerTransport is ITPerCallTransportProvider provider &&
provider.SupportsPerCallTransport;
+
+ /// <summary>
+ /// Creates a framed transport with independent frame buffers around a
new per-call underlying transport.
+ /// </summary>
+ /// <param name="cancellationToken">Token used to cancel underlying
transport creation.</param>
+ /// <returns>A new framed transport owned by the caller.</returns>
+ /// <exception cref="NotSupportedException">The underlying transport
does not support per-call transports.</exception>
+ public async Task<TTransport>
CreatePerCallTransportAsync(CancellationToken cancellationToken)
+ {
+ if (!(InnerTransport is ITPerCallTransportProvider provider) ||
!provider.SupportsPerCallTransport)
+ {
+ throw new NotSupportedException("The underlying transport does
not support per-call transports.");
+ }
+
+ var transport = await
provider.CreatePerCallTransportAsync(cancellationToken);
+ try
+ {
+ return new TFramedTransport(transport);
+ }
+ catch
+ {
+ transport.Dispose();
Review Comment:
A provider that unexpectedly returns `null` causes the constructor to throw,
but this cleanup dereference then masks that failure with a
`NullReferenceException`. The base client explicitly guards against null
provider results, so this wrapper should preserve a meaningful failure too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
