Skip to content

Commit

Permalink
Accurate error propagation from RoGetActivationFactory (#559)
Browse files Browse the repository at this point in the history
  • Loading branch information
kennykerr authored Mar 16, 2020
1 parent f007201 commit b722907
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 19 deletions.
6 changes: 5 additions & 1 deletion strings/base_activation.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ namespace winrt::impl
return 0;
}

com_ptr<IErrorInfo> error_info;
WINRT_IMPL_GetErrorInfo(0, error_info.put_void());

std::wstring path{ static_cast<hstring const&>(name) };
std::size_t count{};

Expand Down Expand Up @@ -97,7 +100,8 @@ namespace winrt::impl
}
}

return error_class_not_available;
WINRT_IMPL_SetErrorInfo(0, error_info.get());
return hr;
}
}

Expand Down
12 changes: 12 additions & 0 deletions strings/base_error.h
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,13 @@ WINRT_EXPORT namespace winrt
hresult_class_not_available(take_ownership_from_abi_t) noexcept : hresult_error(impl::error_class_not_available, take_ownership_from_abi) {}
};

struct hresult_class_not_registered : hresult_error
{
hresult_class_not_registered() noexcept : hresult_error(impl::error_class_not_registered) {}
hresult_class_not_registered(param::hstring const& message) noexcept : hresult_error(impl::error_class_not_registered, message) {}
hresult_class_not_registered(take_ownership_from_abi_t) noexcept : hresult_error(impl::error_class_not_registered, take_ownership_from_abi) {}
};

struct hresult_changed_state : hresult_error
{
hresult_changed_state() noexcept : hresult_error(impl::error_changed_state) {}
Expand Down Expand Up @@ -453,6 +460,11 @@ WINRT_EXPORT namespace winrt
throw hresult_class_not_available(take_ownership_from_abi);
}

if (result == impl::error_class_not_registered)
{
throw hresult_class_not_registered(take_ownership_from_abi);
}

if (result == impl::error_changed_state)
{
throw hresult_changed_state(take_ownership_from_abi);
Expand Down
1 change: 1 addition & 0 deletions strings/base_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ namespace winrt::impl
constexpr hresult error_out_of_bounds{ static_cast<hresult>(0x8000000B) }; // E_BOUNDS
constexpr hresult error_no_interface{ static_cast<hresult>(0x80004002) }; // E_NOINTERFACE
constexpr hresult error_class_not_available{ static_cast<hresult>(0x80040111) }; // CLASS_E_CLASSNOTAVAILABLE
constexpr hresult error_class_not_registered{ static_cast<hresult>(0x80040154) }; // REGDB_E_CLASSNOTREG
constexpr hresult error_changed_state{ static_cast<hresult>(0x8000000C) }; // E_CHANGED_STATE
constexpr hresult error_illegal_method_call{ static_cast<hresult>(0x8000000E) }; // E_ILLEGAL_METHOD_CALL
constexpr hresult error_illegal_state_change{ static_cast<hresult>(0x8000000D) }; // E_ILLEGAL_STATE_CHANGE
Expand Down
12 changes: 2 additions & 10 deletions test/old_tests/UnitTests/get_activation_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,7 @@ TEST_CASE("get_activation_factory")
}

// Calling get_activation_factory with an invalid class name
try
{
get_activation_factory(L"Composable.DoesNotExist");
REQUIRE(false);
}
catch (hresult_class_not_available const& e)
{
REQUIRE(e.message() == L"Composable.DoesNotExist");
}
REQUIRE_THROWS_AS(get_activation_factory(L"Composable.DoesNotExist"), hresult_class_not_registered);
}

TEST_CASE("try_get_activation_factory")
Expand Down Expand Up @@ -72,6 +64,6 @@ TEST_CASE("try_get_activation_factory")
auto factory = try_get_activation_factory<Component::IErrors>(e);
REQUIRE(factory == nullptr);
REQUIRE(get_error_info() == nullptr);
REQUIRE(e.code() == CLASS_E_CLASSNOTAVAILABLE);
REQUIRE(e.code() == REGDB_E_CLASSNOTREG);
}
}
30 changes: 30 additions & 0 deletions test/test/hresult_class_not_registered.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#include "pch.h"

using namespace winrt;
using namespace Windows::Foundation;

namespace
{
IAsyncAction Async()
{
// This is just a simple way of testing all of the ABI and projection
// error propagation in one go.
throw hresult_class_not_registered(L"test message");
}
}
TEST_CASE("hresult_class_not_registered")
{
REQUIRE(hresult_class_not_registered().message() == L"Class not registered");
REQUIRE(hresult_class_not_registered().code() == REGDB_E_CLASSNOTREG);

try
{
Async().get();
FAIL(L"Previous line should throw");
}
catch (hresult_class_not_registered const& e)
{
REQUIRE(e.message() == L"test message");
REQUIRE(e.code() == REGDB_E_CLASSNOTREG);
}
}
1 change: 1 addition & 0 deletions test/test/test.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@
<ClCompile Include="delegates.cpp" />
<ClCompile Include="disconnected.cpp" />
<ClCompile Include="enum.cpp" />
<ClCompile Include="hresult_class_not_registered.cpp" />
<ClCompile Include="error_info.cpp" />
<ClCompile Include="event_deferral.cpp" />
<ClCompile Include="async_local.cpp" />
Expand Down
8 changes: 4 additions & 4 deletions test/test/velocity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,24 @@ TEST_CASE("velocity")
REQUIRE(b == nullptr);

// Class1 is always disabled and thus will not activate.
REQUIRE_THROWS_AS(Class1(), hresult_class_not_available);
REQUIRE_THROWS_AS(Class1(), hresult_class_not_registered);

// Class2 is always enabled so should activate just fine.
Class2 c;
c.Class2_Method();

// Class3 is always disabled and thus will not activate.
REQUIRE_THROWS_AS(Class3(), hresult_class_not_available);
REQUIRE_THROWS_AS(Class3(), hresult_class_not_registered);

// Class4 is not feature-controlled but uses feature interfaces.
Class4 d;
d.Class4_Method();

// The single argument constructor is always disabled.
REQUIRE_THROWS_AS(Class4(1), hresult_class_not_available);
REQUIRE_THROWS_AS(Class4(1), hresult_class_not_registered);

// The Class4_Static1 static is always disabled.
REQUIRE_THROWS_AS(Class4::Class4_Static1(), hresult_class_not_available);
REQUIRE_THROWS_AS(Class4::Class4_Static1(), hresult_class_not_registered);

// The two argument constructor is always enabled.
Class4 e(1, 2);
Expand Down
8 changes: 4 additions & 4 deletions test/test_win7/velocity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,24 @@ TEST_CASE("velocity")
REQUIRE(b == nullptr);

// Class1 is always disabled and thus will not activate.
REQUIRE_THROWS_AS(Class1(), hresult_class_not_available);
REQUIRE_THROWS_AS(Class1(), hresult_class_not_registered);

// Class2 is always enabled so should activate just fine.
Class2 c;
c.Class2_Method();

// Class3 is always disabled and thus will not activate.
REQUIRE_THROWS_AS(Class3(), hresult_class_not_available);
REQUIRE_THROWS_AS(Class3(), hresult_class_not_registered);

// Class4 is not feature-controlled but uses feature interfaces.
Class4 d;
d.Class4_Method();

// The single argument constructor is always disabled.
REQUIRE_THROWS_AS(Class4(1), hresult_class_not_available);
REQUIRE_THROWS_AS(Class4(1), hresult_class_not_registered);

// The Class4_Static1 static is always disabled.
REQUIRE_THROWS_AS(Class4::Class4_Static1(), hresult_class_not_available);
REQUIRE_THROWS_AS(Class4::Class4_Static1(), hresult_class_not_registered);

// The two argument constructor is always enabled.
Class4 e(1, 2);
Expand Down

0 comments on commit b722907

Please sign in to comment.