Skip to content
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

Consider changing IExceptionToErrorInfoConverter namespace #15557

Open
NecatiMeral opened this issue Jan 30, 2023 · 3 comments · May be fixed by #15573
Open

Consider changing IExceptionToErrorInfoConverter namespace #15557

NecatiMeral opened this issue Jan 30, 2023 · 3 comments · May be fixed by #15573
Assignees
Milestone

Comments

@NecatiMeral
Copy link
Contributor

NecatiMeral commented Jan 30, 2023

Hi,

The IExceptionToErrorInfoConverter interface and it's implementation are in the Volo.Abp.ExceptionHandling package (which targets netstandard2.0).
It's misleading that the components are inside the Volo.Abp.AspNetCore.ExceptionHandling namespace since it can be used on all platforms (wpf, winforms, you name it).

I can contribute a PR which refactors the namespace if it's ok. I just wanted to discuss this idea first.

@maliming
Copy link
Member

hi

I agree with you.

@NecatiMeral
Copy link
Contributor Author

NecatiMeral commented Jan 31, 2023

Great, I'll contribute a PR in the upcoming days. 👍

How should I deal with AbpExceptionHandlingOptions?
The property names (SendExceptionsDetailsToClients for example) can be refactored to IncludeExceptionsDetails and IncludeStackTrace.

Should I rename them too (breaking change), implement aliases (compatible properties) or leave them as they are (will update the Docs too)?

@stale
Copy link

stale bot commented Apr 2, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

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

Successfully merging a pull request may close this issue.

4 participants