Skip to content

Create exception formatters for different exceptions #4378

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 12, 2020
Merged

Create exception formatters for different exceptions #4378

merged 1 commit into from
Feb 12, 2020

Conversation

russcam
Copy link
Contributor

@russcam russcam commented Feb 10, 2020

This commit changes the ExceptionFormatterResolver to return a new instance
of the now generic ExceptionFormatter for the type of exception.
This fixes a bug where an ExceptionFormatter of type IJsonFormatter
is returned for derived exception types, resulting in an InvalidCastException
at runtime.

Formatter instances created are cached in ElasticsearchNetFormatterResolver
for the type T, so will be reused once created.

Fixes #4294

This commit changes the ExceptionFormatterResolver to return a new instance
of the now generic ExceptionFormatter<TException> for the type of exception.
This fixes a bug where an ExceptionFormatter of type IJsonFormatter<Exception>
is returned for derived exception types, resulting in an InvalidCastException
at runtime.

Formatter instances created are cached in ElasticsearchNetFormatterResolver
for the type T, so will be reused once created.

Fixes #4294
public IJsonFormatter<T> GetFormatter<T>()
{
if (typeof(Exception).IsAssignableFrom(typeof(T)))
return (IJsonFormatter<T>)ExceptionFormatter;
{
var type = typeof(ExceptionFormatter<>).MakeGenericType(typeof(T));
Copy link
Member

Choose a reason for hiding this comment

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

.CreateGenericInstance()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CreateGenericInstance exists in Nest, but ExceptionFormatter is in Elasticsearch.Net. Can move the extension method to Elasticsearch.Net?

Copy link
Member

Choose a reason for hiding this comment

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

I think we can, we can also follow up in a new issue.

@russcam russcam merged commit 8156ea7 into 7.x Feb 12, 2020
@russcam russcam deleted the fix/4294 branch February 12, 2020 02:42
@russcam
Copy link
Contributor Author

russcam commented Feb 12, 2020

ported to master 2d63ce0

russcam added a commit that referenced this pull request Feb 12, 2020
This commit changes the ExceptionFormatterResolver to return a new instance
of the now generic ExceptionFormatter<TException> for the type of exception.
This fixes a bug where an ExceptionFormatter of type IJsonFormatter<Exception>
is returned for derived exception types, resulting in an InvalidCastException
at runtime.

Formatter instances created are cached in ElasticsearchNetFormatterResolver
for the type T, so will be reused once created.

Fixes #4294

(cherry picked from commit 8156ea7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants