Skip to content

Rename Exception<T> and have non-generic base #573

Closed

Description

The base class for Semantic Kernel exceptions is Exception<TErrorCode>. This is too general a name, even though it's part of the Microsoft.SemanticKernel.Diagnostics namespace. Once a developer imports the namespace, catch blocks like catch (Exception<TErrorCode>) to catch all SK exceptions look much more broad and general-purpose than it is. At the same time, it's not broad enough to catch all SK exceptions, because it'll only catch those for that specific TErrorCode enum type.

Further, the TErrorCode needn't actually be part of the exception's identity. The base exception type doesn't store anything of that type, and instead just accepts an instance as a ctor parameter and uses it to build up a message. That can be done just accepting an object; if you're about to throw an exception, one additional allocation to box an enum value doesn't matter.

It'd be better to have just

public abstract class SemanticKernelException : Exception
{
    protected SemanticKernelException();
    protected SemanticKernelException(string? message);
    protected SemanticKernelException(string? message, Exception? innerException);
}

to address both concerns. Derived exception types can include an error code in whatever message they pass down. If we want a BuildMessage helper to assist with that, it can be an internal / private protected static method on the base exception type.

cc: Devis Lucato (@dluc), Shawn Callegari (@shawncal)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions