Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion strings/base_error.h
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ namespace winrt::impl
{
inline hresult check_hresult_allow_bounds(hresult const result)
{
if (result != impl::error_out_of_bounds)
if (result != impl::error_out_of_bounds && result != impl::error_fail && result != impl::error_file_not_found)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wish there was some way to incentivize the implementations to clean up their act as this isn't sustainable and they're not likely to fix this on their end.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a contract breaking change now if we "fix" the XAML implementation.

{
check_hresult(result);
}
Expand Down
1 change: 1 addition & 0 deletions strings/base_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,4 +147,5 @@ namespace winrt::impl
constexpr hresult error_canceled{ static_cast<hresult>(0x800704C7) }; // HRESULT_FROM_WIN32(ERROR_CANCELLED)
constexpr hresult error_bad_alloc{ static_cast<hresult>(0x8007000E) }; // E_OUTOFMEMORY
constexpr hresult error_not_initialized{ static_cast<hresult>(0x800401F0) }; // CO_E_NOTINITIALIZED
constexpr hresult error_file_not_found{ static_cast<hresult>(0x80070002) }; // HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND)
}
40 changes: 28 additions & 12 deletions test/old_tests/UnitTests/TryLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,21 +110,37 @@ TEST_CASE("TryRemove")

TEST_CASE("TryLookup TryRemove error")
{
// Simulate a non-agile map that is being accessed from the wrong thread.
// "Try" operations should throw rather than erroneously report "not found".
// Because they didn't even try. The operation never got off the ground.
struct incorrectly_used_non_agile_map : implements<incorrectly_used_non_agile_map, IMap<int, int>>
// A map that throws a specific error, used to verify various edge cases.
struct error_map : implements<error_map, IMap<int, int>>
{
int Lookup(int) { throw hresult_wrong_thread(); }
int32_t Size() { throw hresult_wrong_thread(); }
bool HasKey(int) { throw hresult_wrong_thread(); }
IMapView<int, int> GetView() { throw hresult_wrong_thread(); }
bool Insert(int, int) { throw hresult_wrong_thread(); }
void Remove(int) { throw hresult_wrong_thread(); }
void Clear() { throw hresult_wrong_thread(); }
hresult code;
int Lookup(int) { throw_hresult(code); }
int32_t Size() { throw_hresult(E_UNEXPECTED); } // shouldn't be called by the test
bool HasKey(int) { throw_hresult(E_UNEXPECTED); } // shouldn't be called by the test
IMapView<int, int> GetView() { throw_hresult(E_UNEXPECTED); } // shouldn't be called by the test
bool Insert(int, int) { throw_hresult(E_UNEXPECTED); } // shouldn't be called by the test
void Remove(int) { throw_hresult(code); }
void Clear() { throw_hresult(E_UNEXPECTED); } // shouldn't be called by the test
};

auto map = make<incorrectly_used_non_agile_map>();
auto self = make_self<error_map>();
IMap<int, int> map = *self;

// Simulate a non-agile map that is being accessed from the wrong thread.
// "Try" operations should throw rather than erroneously report "not found".
// Because they didn't even try. The operation never got off the ground.
self->code = RPC_E_WRONG_THREAD;
REQUIRE_THROWS_AS(map.TryLookup(123), hresult_wrong_thread);
REQUIRE_THROWS_AS(map.TryRemove(123), hresult_wrong_thread);

// Some implementations return HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND)
// or E_FAIL when the key is not present.
self->code = HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND);
REQUIRE(!map.TryLookup(123));
REQUIRE(!map.TryRemove(123));

self->code = E_FAIL;
REQUIRE(!map.TryLookup(123));
REQUIRE(!map.TryRemove(123));

}