Skip to content

Commit b1f1055

Browse files
evanjzoufacebook-github-bot
authored andcommitted
BiDi works up until transport
Summary: Get bidi to send the response from transport, granted that part is incomplete Reviewed By: sazonovkirill Differential Revision: D83386024 fbshipit-source-id: f3de4fca723d2adf91b6ea03637f85c0bf217cd9
1 parent 049d866 commit b1f1055

8 files changed

Lines changed: 102 additions & 10 deletions

File tree

third-party/thrift/src/thrift/lib/cpp2/async/ReplyInfo.h

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,8 @@ class BiDiStreamReplyInfo {
142142
crc32c_{std::move(crc32c)} {}
143143

144144
void operator()() noexcept {
145-
// TODO(ezou) stubbed
146-
req_->sendErrorWrapped(
147-
folly::make_exception_wrapper<TApplicationException>(
148-
TApplicationException::INTERNAL_ERROR,
149-
"TODO(ezou) BiDiStreamReplyInfo not implemented"),
150-
kUnknownErrorCode);
145+
req_->sendBiDiReply(
146+
std::move(response_), std::move(serverBiDiStreamFactory_), crc32c_);
151147
}
152148

153149
private:

third-party/thrift/src/thrift/lib/cpp2/async/ResponseChannel.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include <thrift/lib/cpp/Thrift.h>
2727
#include <thrift/lib/cpp/server/TServerObserver.h>
2828
#include <thrift/lib/cpp2/async/MessageChannel.h>
29+
#include <thrift/lib/cpp2/async/ServerBiDiStreamFactory.h>
2930
#include <thrift/lib/cpp2/async/ServerStream.h>
3031
#include <thrift/lib/cpp2/async/Sink.h>
3132
#include <thrift/lib/cpp2/async/StreamCallbacks.h>
@@ -147,6 +148,13 @@ class ResponseChannelRequest {
147148
}
148149
#endif
149150

151+
virtual void sendBiDiReply(
152+
ResponsePayload&&,
153+
detail::ServerBiDiStreamFactory&&,
154+
folly::Optional<uint32_t> = folly::none) {
155+
throw std::logic_error("unimplemented");
156+
}
157+
150158
FOLLY_NODISCARD static bool sendBiDiReply(
151159
ResponseChannelRequest::UniquePtr request,
152160
folly::EventBase* eb,

third-party/thrift/src/thrift/lib/cpp2/async/ServerBiDiStreamFactory.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ void ServerBiDiStreamFactory::setInteraction(TilePtr&& interaction) {
2727
interaction_ = std::move(interaction);
2828
}
2929

30+
bool ServerBiDiStreamFactory::valid() const {
31+
// TODO(ezou) this is a stub, revisit this.
32+
return startFunction_ != nullptr;
33+
}
34+
3035
void ServerBiDiStreamFactory::start(
3136
FirstResponsePayload&& payload,
3237
BiDiClientCallback* clientCallback,

third-party/thrift/src/thrift/lib/cpp2/async/ServerBiDiStreamFactory.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,21 @@ class ServerBiDiStreamFactory {
5050
std::shared_ptr<ContextStack>,
5151
TilePtr&&,
5252
FirstResponsePayload&&,
53-
BiDiClientCallback*,
53+
BiDiClientCallback* clientCallback,
5454
folly::EventBase*) mutable -> void {
55-
LOG(FATAL) << "Not implemented";
55+
clientCallback->onFirstResponseError(
56+
folly::make_exception_wrapper<rocket::RocketException>(
57+
rocket::ErrorCode::APPLICATION_ERROR,
58+
"TODO(ezou) in Transport not implemented"));
5659
};
5760
}
5861

5962
void setContextStack(std::shared_ptr<ContextStack> contextStack);
6063

6164
void setInteraction(TilePtr&& interaction);
6265

66+
bool valid() const;
67+
6368
void start(
6469
FirstResponsePayload&& firstResponsePayload,
6570
BiDiClientCallback* clientCallback,

third-party/thrift/src/thrift/lib/cpp2/async/tests/BiDiServiceE2ETest.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ CO_TEST_F(BiDiServiceTest, BiDiNoResponse) {
7979
co_await client->co_echo();
8080
CO_FAIL() << "No error - error expected";
8181
} catch (TApplicationException e) {
82-
EXPECT_EQ(e.getMessage(), "TODO(ezou) BiDiStreamReplyInfo not implemented");
82+
EXPECT_EQ(e.getMessage(), "Unexpected error frame type: 513");
8383
}
8484
}
8585

@@ -89,7 +89,7 @@ CO_TEST_F(BiDiServiceTest, BiDiWithResponse) {
8989
co_await client->co_echoWithResponse("Test");
9090
CO_FAIL() << "No error - error expected";
9191
} catch (TApplicationException e) {
92-
EXPECT_EQ(e.getMessage(), "TODO(ezou) BiDiStreamReplyInfo not implemented");
92+
EXPECT_EQ(e.getMessage(), "Unexpected error frame type: 513");
9393
}
9494
}
9595

third-party/thrift/src/thrift/lib/cpp2/transport/core/ThriftRequest.h

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,30 @@ class ThriftRequestCore : public ResponseChannelRequest {
250250
}
251251
#endif
252252

253+
void sendBiDiReply(
254+
ResponsePayload&& response,
255+
detail::ServerBiDiStreamFactory&& bidiStreamFactory,
256+
folly::Optional<uint32_t> crc32c) final {
257+
if (tryCancel()) {
258+
cancelTimeout();
259+
auto metadata = makeResponseRpcMetadata(
260+
header_.extractAllWriteHeaders(),
261+
header_.extractProxiedPayloadMetadata(),
262+
header_.getChecksum());
263+
if (crc32c) {
264+
metadata.crc32c() = *crc32c;
265+
}
266+
sendReplyInternal(
267+
std::move(metadata),
268+
std::move(response).buffer(),
269+
std::move(bidiStreamFactory));
270+
271+
if (auto* observer = serverConfigs_.getObserver()) {
272+
observer->sentReply();
273+
}
274+
}
275+
}
276+
253277
bool sendBiDiReply(
254278
ResponsePayload&& response,
255279
BiDiServerCallbackPtr callback,
@@ -393,6 +417,13 @@ class ThriftRequestCore : public ResponseChannelRequest {
393417
}
394418
#endif
395419

420+
virtual void sendBiDiThriftResponse(
421+
ResponseRpcMetadata&&,
422+
std::unique_ptr<folly::IOBuf>,
423+
detail::ServerBiDiStreamFactory&&) noexcept {
424+
LOG(FATAL) << "sendBiDiThriftResponse not implemented";
425+
}
426+
396427
virtual bool sendBiDiThriftResponse(
397428
ResponseRpcMetadata&&,
398429
std::unique_ptr<folly::IOBuf>,
@@ -508,6 +539,18 @@ class ThriftRequestCore : public ResponseChannelRequest {
508539
}
509540
#endif
510541

542+
void sendReplyInternal(
543+
ResponseRpcMetadata&& metadata,
544+
std::unique_ptr<folly::IOBuf> buf,
545+
detail::ServerBiDiStreamFactory&& bidiStreamFactory) {
546+
if (checkResponseSize(*buf)) {
547+
sendBiDiThriftResponse(
548+
std::move(metadata), std::move(buf), std::move(bidiStreamFactory));
549+
} else {
550+
sendResponseTooBigEx();
551+
}
552+
}
553+
511554
bool sendReplyInternal(
512555
ResponseRpcMetadata&& metadata,
513556
std::unique_ptr<folly::IOBuf> buf,

third-party/thrift/src/thrift/lib/cpp2/transport/rocket/server/RocketThriftRequests.cpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -881,6 +881,36 @@ void ThriftServerRequestBiDi::sendSerializedError(
881881
FirstResponsePayload(std::move(exbuf), std::move(metadata))));
882882
}
883883

884+
void ThriftServerRequestBiDi::sendBiDiThriftResponse(
885+
ResponseRpcMetadata&& metadata,
886+
std::unique_ptr<folly::IOBuf> data,
887+
apache::thrift::detail::ServerBiDiStreamFactory&&
888+
bidiStreamFactory) noexcept {
889+
if (!bidiStreamFactory.valid()) {
890+
sendSerializedError(std::move(metadata), std::move(data));
891+
return;
892+
}
893+
894+
if (auto responseRpcError = processFirstResponse(
895+
metadata, data, getProtoId(), version_, getCompressionConfig())) {
896+
auto ex = makeRocketException(
897+
*responseRpcError, *context_.connection().getPayloadSerializer());
898+
handleStreamError(std::move(ex), clientCallback_);
899+
return;
900+
}
901+
902+
context_.unsetMarkRequestComplete();
903+
// Missing some configuration here
904+
// clientCallback_->setProtoId(getProtoId());
905+
// clientCallback_->setChunkTimeout(sinkConsumer.chunkTimeout);
906+
auto payload = apache::thrift::FirstResponsePayload{
907+
std::move(data), std::move(metadata)};
908+
payload.fds =
909+
std::move(getRequestContext()->getHeader()->fds.dcheckToSendOrEmpty());
910+
std::move(bidiStreamFactory)
911+
.start(std::move(payload), clientCallback_, getEventBase());
912+
}
913+
884914
bool ThriftServerRequestBiDi::sendBiDiThriftResponse(
885915
ResponseRpcMetadata&& metadata,
886916
std::unique_ptr<folly::IOBuf> data,

third-party/thrift/src/thrift/lib/cpp2/transport/rocket/server/RocketThriftRequests.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,11 @@ class ThriftServerRequestBiDi final : public RocketThriftRequest {
353353
void sendSerializedError(
354354
ResponseRpcMetadata&&, std::unique_ptr<folly::IOBuf>) noexcept override;
355355

356+
void sendBiDiThriftResponse(
357+
ResponseRpcMetadata&&,
358+
std::unique_ptr<folly::IOBuf>,
359+
apache::thrift::detail::ServerBiDiStreamFactory&&) noexcept override;
360+
356361
bool sendBiDiThriftResponse(
357362
ResponseRpcMetadata&&,
358363
std::unique_ptr<folly::IOBuf>,

0 commit comments

Comments
 (0)