From a84c436f256fa8792c1d1c50a445197d0b8f7bd5 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 30 Nov 2017 22:56:56 +0000 Subject: [PATCH] Fix ProtocolError single-argument (subtype only) constructor Correctly initialise mType from the ErrorType CONSTANT in the protocol message definition in this case, instead of leaving it uninitialised. Add test. Also test that these unexpected errors cause the connection to be closed as expected. --- lib/server/makeprotocol.pl.in | 33 +++++++++++++++++-------- test/basicserver/TestCommands.cpp | 13 ++++++++++ test/basicserver/TestProtocol.txt | 5 ++++ test/basicserver/testbasicserver.cpp | 37 +++++++++++++++++++++++++--- 4 files changed, 74 insertions(+), 14 deletions(-) diff --git a/lib/server/makeprotocol.pl.in b/lib/server/makeprotocol.pl.in index 197821906..6488499ed 100755 --- a/lib/server/makeprotocol.pl.in +++ b/lib/server/makeprotocol.pl.in @@ -118,11 +118,11 @@ while() my ($type,$name,$value) = split /\s+/,$l; if($type eq 'CONSTANT') { - push @{$cmd_constants{$current_cmd}},"$name = $value" + push @{$cmd_constants{$current_cmd}}, [$name, $value]; } else { - push @{$cmd_contents{$current_cmd}},$type,$name; + push @{$cmd_contents{$current_cmd}}, $type, $name; } } else @@ -317,8 +317,11 @@ __E # constants if(exists $cmd_constants{$cmd}) { - print H "\tenum\n\t{\n\t\t"; - print H join(",\n\t\t",@{$cmd_constants{$cmd}}); + print H "\tenum\n\t{\n"; + foreach my $constant (@{$cmd_constants{$cmd}}) + { + print H "\t\t", $constant->[0], " = ", $constant->[1], ",\n"; + } print H "\n\t};\n"; } @@ -332,9 +335,19 @@ __E { $error_message = $cmd; my ($mem_type,$mem_subtype) = split /,/,obj_get_type_params($cmd,'IsError'); - my $error_type = $cmd_constants{"ErrorType"}; + my %constants_hash; + my $error_type_value = ""; + + foreach my $constant (@{$cmd_constants{$cmd}}) + { + if($constant->[0] eq "ErrorType") + { + $error_type_value = $constant->[0]; + } + } + print H <<__E; - $cmd_class(int SubType) : m$mem_type($error_type), m$mem_subtype(SubType) { } + $cmd_class(int SubType) : m$mem_type($error_type_value), m$mem_subtype(SubType) { } bool IsError(int &rTypeOut, int &rSubTypeOut) const; std::string GetMessage() const { return GetMessage(m$mem_subtype); }; static std::string GetMessage(int subtype); @@ -515,13 +528,13 @@ std::string $cmd_class\::GetMessage(int subtype) switch(subtype) { __E - foreach my $const (@{$cmd_constants{$cmd}}) + foreach my $constant (@{$cmd_constants{$cmd}}) { - next unless $const =~ /^Err_(.*)/; + my $enum_name = $constant->[0]; + next unless $enum_name =~ /^Err_(.*)/; my $shortname = $1; - $const =~ s/ = .*//; print CPP <<__E; - case $const: return "$shortname"; + case $enum_name: return "$shortname"; __E } print CPP <<__E; diff --git a/test/basicserver/TestCommands.cpp b/test/basicserver/TestCommands.cpp index bdbdffeb8..51b46545f 100644 --- a/test/basicserver/TestCommands.cpp +++ b/test/basicserver/TestCommands.cpp @@ -106,3 +106,16 @@ std::auto_ptr TestProtocolString::DoCommand(TestProtocolRep return std::auto_ptr(new TestProtocolString(mTest)); } +std::auto_ptr TestProtocolDeliberateError::DoCommand( + TestProtocolReplyable &rProtocol, TestContext &rContext) const +{ + return std::auto_ptr(new TestProtocolError( + TestProtocolError::ErrorType, + TestProtocolError::Err_DeliberateError)); +} + +std::auto_ptr TestProtocolUnexpectedError::DoCommand( + TestProtocolReplyable &rProtocol, TestContext &rContext) const +{ + THROW_EXCEPTION_MESSAGE(CommonException, Internal, "unexpected error"); +} diff --git a/test/basicserver/TestProtocol.txt b/test/basicserver/TestProtocol.txt index 5bca9f49b..87d9f7cc0 100644 --- a/test/basicserver/TestProtocol.txt +++ b/test/basicserver/TestProtocol.txt @@ -9,6 +9,8 @@ BEGIN_OBJECTS Error 0 IsError(Type,SubType) Reply int32 Type int32 SubType + CONSTANT ErrorType 1000 + CONSTANT Err_DeliberateError 1 Hello 1 Command(Hello) Reply int32 Number32 @@ -40,3 +42,6 @@ SendStream 8 Command(GetStream) StreamWithCommand String 9 Command(String) Reply string Test +DeliberateError 10 Command(String) + +UnexpectedError 11 Command(String) diff --git a/test/basicserver/testbasicserver.cpp b/test/basicserver/testbasicserver.cpp index 8595fc11c..8869f4b54 100644 --- a/test/basicserver/testbasicserver.cpp +++ b/test/basicserver/testbasicserver.cpp @@ -524,6 +524,11 @@ int test(int argc, const char *argv[]) } } + #ifndef WIN32 + // Don't die quietly if the server dies while we're communicating with it + signal(SIGPIPE, SIG_IGN); + #endif + //printf("SKIPPING TESTS------------------------\n"); //goto protocolserver; @@ -801,11 +806,35 @@ int test(int argc, const char *argv[]) TEST_THAT(reply->GetNumber8() == 22); TEST_THAT(reply->GetText() == "Hello world!"); } + + // Try to trigger a deliberate and an unexpected error (the latter + // by throwing an exception that isn't caught and handled by + // HandleException): + { + TEST_CHECK_THROWS(protocol.QueryDeliberateError(), + ConnectionException, Protocol_UnexpectedReply); + int type, subtype; + protocol.GetLastError(type, subtype); + TEST_EQUAL(TestProtocolError::ErrorType, type); + TEST_EQUAL(TestProtocolError::Err_DeliberateError, subtype); + } + + { + TEST_CHECK_THROWS(protocol.QueryUnexpectedError(), + ConnectionException, Protocol_UnexpectedReply); + int type, subtype; + protocol.GetLastError(type, subtype); + TEST_EQUAL(TestProtocolError::ErrorType, type); + TEST_EQUAL(-1, subtype); + } + + // The unexpected exception should kill the server child process that we + // connected to (except on Windows where the server does not fork a child), + // so we cannot communicate with it any more: + TEST_CHECK_THROWS(protocol.QueryQuit(), + ConnectionException, SocketWriteError); - // Quit query to finish - protocol.QueryQuit(); - - // Kill it + // Kill the main server process: TEST_THAT(KillServer(pid)); ::sleep(1); TEST_THAT(!ServerIsAlive(pid));