Changes between Initial Version and Version 1 of MapGuideRfc183

Timestamp:

Oct 29, 2020, 1:40:43 AM (3 years ago)

Author:

jng

Comment:

--

MapGuideRfc183

@@ +1 @@
+
+= !MapGuide RFC 183 - Flatten exception hierarchy =
+
+This page contains a change request (RFC) for the !MapGuide Open Source project.
+More !MapGuide RFCs can be found on the [wiki:MapGuideRfcs RFCs] page.
+
+== Status ==
+
+||RFC Template Version||(1.0)||
+||Submission Date||29 Oct 2020||
+||Last Modified||29 Oct 2020||
+||Author||Jackie Ng||
+||RFC Status||draft||
+||Implementation Status||||
+||Proposed Milestone||4.0||
+||Assigned PSC guide(s)||(when determined)||
+||'''Voting History'''||(vote date)||
+||+1||||
+||+0||||
+||-0||||
+||-1||||
+||no vote|| ||
+
+== Overview ==
+
+This RFC proposes to flatten the MapGuide exception hierarchy to just the `MgException` class
+
+== Motivation ==
+
+Currently, the full MapGuide exception class hierarchy is replicated out to the respective Java/PHP/.net wrapper APIs.
+
+The upcoming work to generate an updated set of wrapper APIs (in particular, for PHP7) with vanilla SWIG has hit some several roadblocks in wrapping the MapGuide classes. One of the roadblocks concerns trying to replicate the current MapGuide exception hierarchy in PHP7.
+
+Also in our various developer samples and documentation, we never really tell developers to catch anything *other* than {{{MgException}}}
+
+To simplify our wrapping process and developer experience, we can flatten the MapGuide exception hierarchy to just {{{MgException}}}
+
+== Proposed Solution ==
+
+Update the `MgException` class to no longer be abstract and include the following internal APIs (full class omitted, only new additions shown):
+
+{{{
+///////////////////////////////////////////////////////////////////////////////
+/// \brief
+/// Base class for exceptions.
+///
+class MG_FOUNDATION_API MgException : public MgSerializable
+{
+PUBLISHED_API:
+
+    ///////////////////////////////////////////////////////////////////////////
+    /// \brief
+    /// Retrieve the exception code for this exception
+    ///
+    /// <!-- Syntax in .Net, Java, and PHP -->
+    /// \htmlinclude DotNetSyntaxTop.html
+    /// string GetExceptionCode();
+    /// \htmlinclude SyntaxBottom.html
+    /// \htmlinclude JavaSyntaxTop.html
+    /// String GetExceptionCode();
+    /// \htmlinclude SyntaxBottom.html
+    /// \htmlinclude PHPSyntaxTop.html
+    /// string GetExceptionCode();
+    /// \htmlinclude SyntaxBottom.html
+    ///
+    /// \return
+    /// The exception code
+    ///
+    /// \since 4.0
+    ///
+    STRING GetExceptionCode();
+
+    ///////////////////////////////////////////////////////////////////////////
+    /// \brief
+    /// Retrieve the native error code that may be set by errors from thirdparty
+    /// libraries
+    ///
+    /// <!-- Syntax in .Net, Java, and PHP -->
+    /// \htmlinclude DotNetSyntaxTop.html
+    /// string GetNativeErrorCode();
+    /// \htmlinclude SyntaxBottom.html
+    /// \htmlinclude JavaSyntaxTop.html
+    /// String GetNativeErrorCode();
+    /// \htmlinclude SyntaxBottom.html
+    /// \htmlinclude PHPSyntaxTop.html
+    /// string GetNativeErrorCode();
+    /// \htmlinclude SyntaxBottom.html
+    ///
+    /// \return
+    /// The native error code
+    ///
+    /// \since 4.0
+    ///
+
+INTERNAL_API:
+
+    ///////////////////////////////////////////////////////////////////////////
+    /// \brief
+    /// Create a MgException from mapping a standard CPP exception to a MgException.
+    ///
+    static MgException* Create(std::exception& stdLibException, CREFSTRING methodName, INT32 lineNumber, CREFSTRING fileName) throw();
+
+    ///////////////////////////////////////////////////////////////////////////
+    /// \brief
+    /// Sets the native error code
+    /// 
+    /// \param nativeErrorCode
+    /// The native error code
+    ///
+    void SetNativeErrorCode(INT64 nativeErrorCode);
+
+protected:
+
+    /// The exception code
+    STRING m_code;
+
+    /// The native error code
+    INT64 m_nativeErrorCode;
+
+INTERNAL_API:
+
+    ///////////////////////////////////////////////////////////////////////////
+    /// \brief
+    /// Construct the object.
+    ///
+    /// \param exceptionCode
+    /// The exception code
+    /// \param methodName
+    /// Name of the method where the exception occurred.
+    /// \param lineNumber
+    /// Line number where the exception occurred.
+    /// \param fileName
+    /// File name where the exception occurred.
+    /// \param whatArguments
+    /// Collection of arguments used to format the message that describes what the exception is.
+    /// \param whyMessageId
+    /// ID of the message that describes why the exception occurs.
+    /// \param whyArguments
+    /// Collection of arguments used to format the message that describes why the exception occurs.
+    ///
+    MgException(CREFSTRING exceptionCode, CREFSTRING methodName, INT32 lineNumber,
+        CREFSTRING fileName, MgStringCollection* whatArguments,
+        CREFSTRING whyMessageId, MgStringCollection* whyArguments) throw();
+
+};
+}}}
+
+The key points with these changes are that:
+
+ * `MgException` is no longer abstract
+ * Instead of throwing a specific subclass of `MgException`, code will simply throw a `MgException` with the appropriate exception code (accessible by the new `GetExceptionCode()` API). A new `MgExceptionCodes` string constant class contains all the valid exception codes that can be specified.
+ * For code that needs to re-map an existing `std::exception` type, they will now throw a `MgException` created from the `MgException::Create()` static method
+ * For code that needs to surface an error code from an external library (eg. FDO, berkeley db, dbxml), they can use the new `SetNativeErrorCode()` method to set the error code, which MapGuide applications can get throw the new `GetNativeErrorCode()` API.
+
+All subclasses of {{{MgException}}} will be removed.
+
+All references to these headers from our SWIG generation files.
+
+Replace all usages where {{{MgException}}} subclasess are thrown to throw {{{MgException}}} instead and pass the original exception name as the error code. For applicable error cases (eg. Errors from FDO, berkeley db, dbxml), the native error code is also set on the exception before throwing.
+
+== Implications ==
+
+If developers have been catching exception types other than {{{MgException}}}, they will have to refactor their code so that it catches {{{MgException}}} and check the result of {{{GetExceptionCode()}}} to determine the appropriate course of action to take.
+
+For example, a catch block like this in .net
+
+{{{
+
+try {
+
+} catch (MgUnauthorizedAccessException ex) {
+    // Handle unauthorized acceess
+} catch (MgSessionExpiredException ex) {
+    // Handle session expired
+} catch (MgException ex) {
+    // Handle general errors
+}
+
+}}}
+
+Will need to be refactored to this
+
+{{{
+
+try {
+
+} catch (MgException ex) {
+    if (ex.GetExceptionCode() == MgExceptionCodes.MgUnauthorizedAccessException) {
+        // Handle unauthorized acceess
+    } else if (ex.GetExceptionCode() == MgExceptionCodes.MgSessionExpiredException) {
+        // Handle session expired
+    } else {
+        // Handle general errors
+    }
+}
+
+}}}
+
+== Test Plan ==
+
+Test APIs that throw exceptions now catch as `MgException` with the right exception code
+
+== Funding / Resources ==
+
+Community