[mapguide-commits] r10252 - in branches/4.0/MgDev: Common/Foundation/Exception Common/Foundation/System Server/src/UnitTesting
svn_mapguide at osgeo.org
svn_mapguide at osgeo.org
Mon Nov 2 20:05:51 PST 2026
Author: jng
Date: 2026-11-02 20:05:50 -0800 (Mon, 02 Nov 2026)
New Revision: 10252
Modified:
branches/4.0/MgDev/Common/Foundation/Exception/Exception.cpp
branches/4.0/MgDev/Common/Foundation/System/Resources.cpp
branches/4.0/MgDev/Common/Foundation/System/Resources.h
branches/4.0/MgDev/Server/src/UnitTesting/TestMisc.cpp
Log:
Explain localized exception message formatting failures
Preserve the exception raised during message lookup or formatting and include a
diagnostic in the fallback message. Report missing resource details or required
and supplied argument counts, along with the original exception's throw site.
Factor placeholder counting into MgResources and cover both failure cases in the
Misc test scope.
Verified with Release and Debug server builds and Misc tests (17 test cases,
218 assertions). The Debug CRT leak report matched the baseline.
Fixes #2867
Modified: branches/4.0/MgDev/Common/Foundation/Exception/Exception.cpp
===================================================================
--- branches/4.0/MgDev/Common/Foundation/Exception/Exception.cpp 2026-11-03 04:01:24 UTC (rev 10251)
+++ branches/4.0/MgDev/Common/Foundation/Exception/Exception.cpp 2026-11-03 04:05:50 UTC (rev 10252)
@@ -271,6 +271,94 @@
///////////////////////////////////////////////////////////////////////////////
/// \brief
+/// Format the location at which an exception was thrown.
+///
+/// The first frame recorded on an exception is the one passed to its
+/// constructor, so it names the method and line which threw it.
+///
+static STRING FormatThrownAt(const MgStringCollection& methodNames,
+ const MgStringCollection& lineNumbers, const MgStringCollection& fileNames)
+{
+ STRING location;
+
+ if (methodNames.GetCount() > 0 && lineNumbers.GetCount() > 0 && fileNames.GetCount() > 0)
+ {
+ location = methodNames.GetItem(0);
+ location += L" (";
+ location += fileNames.GetItem(0);
+ location += L":";
+ location += lineNumbers.GetItem(0);
+ location += L")";
+ }
+
+ return location;
+}
+
+///////////////////////////////////////////////////////////////////////////////
+/// \brief
+/// Format the reason why a message resource could not be formatted.
+///
+/// This is appended to the "failed to retrieve message" fallback, so that a
+/// message which cannot be formatted explains itself instead of having its
+/// reason discarded.
+///
+/// No message resource is formatted here, because the caller has already
+/// failed to do that. Everything reported comes from the failed lookup and
+/// from the exceptions involved, so this cannot fail and cannot recurse back
+/// into the formatter it is reporting on.
+///
+static STRING FormatMessageFailure(CREFSTRING locale, CREFSTRING section,
+ CREFSTRING resourceId, MgStringCollection* arguments, CREFSTRING resourceStr,
+ CREFSTRING cause, CREFSTRING thrownAt)
+{
+ STRING reason;
+
+ if (resourceStr.empty())
+ {
+ reason = L"The message resource \"";
+ reason += resourceId;
+ reason += L"\" was not found in section \"";
+ reason += section;
+ reason += L"\" for locale \"";
+ reason += locale;
+ reason += L"\".";
+ }
+ else
+ {
+ STRING required;
+ STRING supplied;
+
+ MgUtil::Int32ToString(MgResources::GetRequiredArgumentCount(resourceStr), required);
+ MgUtil::Int32ToString(NULL != arguments ? arguments->GetCount() : 0, supplied);
+
+ reason = L"The message resource \"";
+ reason += resourceId;
+ reason += L"\" requires ";
+ reason += required;
+ reason += L" argument(s) but ";
+ reason += supplied;
+ reason += L" were supplied.";
+ }
+
+ if (!cause.empty())
+ {
+ reason += L" ";
+ reason += cause;
+ reason += L".";
+ }
+
+ if (!thrownAt.empty())
+ {
+ reason += L" Exception thrown in ";
+ reason += thrownAt;
+ reason += L".";
+ }
+
+ return reason;
+}
+
+///////////////////////////////////////////////////////////////////////////////
+/// \brief
/// Format a message using the specified locale.
///
STRING MgException::FormatMessage(CREFSTRING locale, CREFSTRING section,
@@ -277,6 +365,10 @@
CREFSTRING resourceId, MgStringCollection* arguments) throw()
{
STRING message;
+ STRING failure;
+ STRING resourceStr;
+ STRING attemptedLocale = locale;
+ Ptr<MgException> cause;
MgResources* resources = NULL;
MG_TRY()
@@ -283,59 +375,41 @@
resources = MgResources::GetInstance();
+ MG_CATCH_AND_RELEASE()
+
if (NULL != resources)
{
// try using the supplied locale
- STRING resourceStr = resources->GetStringResource(locale, section,
- resourceId);
+ MG_TRY()
- message = resources->FormatMessage(resourceStr, arguments);
- }
+ resourceStr = resources->GetStringResource(locale, section, resourceId);
- MG_CATCH_AND_RELEASE()
-#if _DEBUG
- if (nullptr != mgException.p)
- {
- // TODO: Ideally we want to be able to capture this exception message and pass it on as the "reason"
- // for why we're failing to retrieve the original exception message. But my initial attempts at this
- // resulted in massive recursively nested error messages due to us:
- //
- // a) Currently being in the exception message formatter
- // b) Naively trying to call GetExceptionMessage() on this exception which has a very high chance of
- // initiating a long cascading recursive chain of similar errors due to argument mismatches on each
- // recursive iteration
- //
- // This is just a variant of the "what do we do if the exception handler itself throws an exception?"
- // problem with some extra recursion thrown in.
- //
- // In the absence of an actual solution, the next best thing is just to stick a breakpoint in this block
- // and inspect the contents of the caught mgException in a debugger.
- //
- // Fortunately, most of the time the actual reason is we are throwing a MgException somewhere but not
- // passing the correct amount of arguments for the exception message. So your breakpoint here hits in a
- // debugger, you know that it's most likely because of this case.
- //
- // Or the alternative reason is the obvious mapguide_$locale.res string bundle was not found at the
- // expected location.
- }
-#endif
- if (message.empty() && NULL != resources && locale != MgResources::DefaultMessageLocale)
- {
- MG_TRY()
+ if (!resourceStr.empty())
+ {
+ message = resources->FormatMessage(resourceStr, arguments);
+ }
- // try using the default locale as a backup
- STRING resourceStr = resources->GetStringResource(MgResources::DefaultMessageLocale, section,
- resourceId);
+ MG_CATCH_AND_RELEASE()
- message = resources->FormatMessage(resourceStr, arguments);
+ cause = mgException;
- MG_CATCH_AND_RELEASE()
-#if _DEBUG
- if (nullptr != mgException.p)
+ if (message.empty() && locale != MgResources::DefaultMessageLocale)
{
- // TODO: See above TODO
+ // try using the default locale as a backup
+ MG_TRY()
+
+ attemptedLocale = MgResources::DefaultMessageLocale;
+ resourceStr = resources->GetStringResource(attemptedLocale, section, resourceId);
+
+ if (!resourceStr.empty())
+ {
+ message = resources->FormatMessage(resourceStr, arguments);
+ }
+
+ MG_CATCH_AND_RELEASE()
+
+ cause = mgException;
}
-#endif
}
if (message.empty() && NULL != resources)
@@ -342,6 +416,30 @@
{
MG_TRY()
+ STRING causeDescription;
+
+ // A message resource which was found but could not be formatted has a
+ // known reason, so only name the exception when the lookup itself is
+ // what failed. Otherwise the "cause" is the formatter rejecting the
+ // arguments, which the reason already reports.
+ if (resourceStr.empty() && NULL != cause.p)
+ {
+ causeDescription = cause->GetExceptionCode();
+
+ STRING causeThrownAt = FormatThrownAt(cause->m_methodNames,
+ cause->m_lineNumbers, cause->m_fileNames);
+
+ if (!causeThrownAt.empty())
+ {
+ causeDescription += L" was thrown in ";
+ causeDescription += causeThrownAt;
+ }
+ }
+
+ failure = FormatMessageFailure(attemptedLocale, section, resourceId,
+ arguments, resourceStr, causeDescription,
+ FormatThrownAt(m_methodNames, m_lineNumbers, m_fileNames));
+
MgStringCollection resourceArguments;
resourceArguments.Add(resourceId);
@@ -348,6 +446,12 @@
message = resources->FormatMessage(
MgResources::FailedToRetrieveMessage, &resourceArguments);
+ if (!failure.empty())
+ {
+ message += L" ";
+ message += failure;
+ }
+
MG_CATCH_AND_RELEASE()
}
Modified: branches/4.0/MgDev/Common/Foundation/System/Resources.cpp
===================================================================
--- branches/4.0/MgDev/Common/Foundation/System/Resources.cpp 2026-11-03 04:01:24 UTC (rev 10251)
+++ branches/4.0/MgDev/Common/Foundation/System/Resources.cpp 2026-11-03 04:05:50 UTC (rev 10252)
@@ -451,6 +451,29 @@
}
+INT32 MgResources::GetRequiredArgumentCount(CREFSTRING stringResource)
+{
+ wchar_t buff[4];
+
+ for (INT32 i = 9; i > 0; i--)
+ {
+ ACE_OS::itoa(i, buff, 10);
+
+ STRING currentReplace = L"%";
+ currentReplace += buff;
+
+ // if placeholder for argument i exists then i
+ // is the number of arguments needed.
+ if (stringResource.find(currentReplace, 0) != string::npos)
+ {
+ return i;
+ }
+ }
+
+ return 0;
+}
+
+
STRING MgResources::FormatMessage(CREFSTRING stringResource, MgStringCollection* arguments)
{
STRING value = stringResource;
@@ -473,22 +496,8 @@
}
// Determine the number of arguments needed
- for (i = 9; i > 0; i--)
- {
- ACE_OS::itoa(i, buff, 10);
- currentReplace = L"%";
- currentReplace += buff;
- pos = stringResource.find(currentReplace, 0);
+ numArgsNeeded = GetRequiredArgumentCount(stringResource);
- // if placeholder for argument i exists then i
- // is the number of arguments needed.
- if (pos != string::npos)
- {
- numArgsNeeded = i;
- break;
- }
- }
-
// If the argument collection size is valid, format message
if ((numArgsProvided == numArgsNeeded) && (numArgsProvided <= 9))
{
Modified: branches/4.0/MgDev/Common/Foundation/System/Resources.h
===================================================================
--- branches/4.0/MgDev/Common/Foundation/System/Resources.h 2026-11-03 04:01:24 UTC (rev 10251)
+++ branches/4.0/MgDev/Common/Foundation/System/Resources.h 2026-11-03 04:05:50 UTC (rev 10252)
@@ -125,6 +125,20 @@
STRING GetStringResource(CREFSTRING locale, CREFSTRING section, CREFSTRING resourceId);
/// \brief
+ /// Get the number of arguments a string resource requires to be formatted.
+ ///
+ /// This is the highest argument placeholder (%1 to %9) present in the
+ /// string resource. A resource with no placeholders requires no arguments.
+ ///
+ /// \param stringResource
+ /// The STRING to be examined. ie. "%1 %2"
+ ///
+ /// \return
+ /// The number of arguments the string resource requires
+ ///
+ static INT32 GetRequiredArgumentCount(CREFSTRING stringResource);
+
+ /// \brief
/// Format a string resource that was retrieved using GetStringResource. The number of arguments
/// needed to be replaced in the stringResource must match the size of the collection given.
///
Modified: branches/4.0/MgDev/Server/src/UnitTesting/TestMisc.cpp
===================================================================
--- branches/4.0/MgDev/Server/src/UnitTesting/TestMisc.cpp 2026-11-03 04:01:24 UTC (rev 10251)
+++ branches/4.0/MgDev/Server/src/UnitTesting/TestMisc.cpp 2026-11-03 04:05:50 UTC (rev 10252)
@@ -83,6 +83,44 @@
}
}
+TEST_CASE("ExceptionMessageFailureReason", "[Misc]")
+{
+ // An exception whose message resource does not exist must say so, and must
+ // name the method which threw it.
+ {
+ Ptr<MgException> ex = new MgException(L"MgThisExceptionCodeDoesNotExist",
+ L"TestMisc::TestCase_ExceptionMessageFailureReason", __LINE__, __WFILE__,
+ nullptr, L"", nullptr);
+
+ STRING msg = ex->GetExceptionMessage(TestServiceFactory::TEST_LOCALE);
+
+ REQUIRE(msg.find(L"Failed to retrieve message for \"MgThisExceptionCodeDoesNotExist\"") != STRING::npos);
+ REQUIRE(msg.find(L"was not found in section \"Exceptions\"") != STRING::npos);
+ REQUIRE(msg.find(L"TestMisc::TestCase_ExceptionMessageFailureReason") != STRING::npos);
+ }
+
+ // An exception thrown with more message arguments than its message resource
+ // takes must report how many it takes and how many were supplied.
+ {
+ Ptr<MgStringCollection> arguments = new MgStringCollection();
+ arguments->Add(L"1");
+ arguments->Add(L"a");
+ arguments->Add(L"2");
+ arguments->Add(L"b");
+
+ Ptr<MgException> ex = new MgException(MgExceptionCodes::MgInvalidArgumentException,
+ L"TestMisc::TestCase_ExceptionMessageFailureReason", __LINE__, __WFILE__,
+ arguments, L"MgResourcesIdentical", NULL);
+
+ STRING msg = ex->GetExceptionMessage(TestServiceFactory::TEST_LOCALE);
+
+ REQUIRE(msg.find(L"Failed to retrieve message for \"MgInvalidArgumentException\"") != STRING::npos);
+ REQUIRE(msg.find(L"requires 0 argument(s) but 4 were supplied") != STRING::npos);
+ REQUIRE(msg.find(L"TestMisc::TestCase_ExceptionMessageFailureReason") != STRING::npos);
+ REQUIRE(msg.find(L"The resources cannot be the same.") != STRING::npos);
+ }
+}
+
TEST_CASE("CommonExceptionMessages", "[Misc]")
{
try
More information about the mapguide-commits
mailing list