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

FaultException is not marked Serializable #3984

Closed
wouterroos opened this issue Oct 22, 2019 · 16 comments
Closed

FaultException is not marked Serializable #3984

wouterroos opened this issue Oct 22, 2019 · 16 comments
Assignees
Labels
feature request Adding new functionality requiring adding an API to the public contract. priority 2 Stack ranked level of priority. P2
Milestone

Comments

@wouterroos
Copy link

wouterroos commented Oct 22, 2019

Describe the bug
When attempting to port code running the .NETFramework code that uses (and serializes) FaultException I ran into the issue that FaultException is not marked as Serializable in System.ServiceModel.Primitives:

public class FaultException : CommunicationException

Is this by design? If I look at the System.ServiceModel reference source the attribute is present (https://github.com/microsoft/referencesource/blob/master/System.ServiceModel/System/ServiceModel/FaultException.cs#L21) and netstandard/core does seem to support it just fine (https://docs.microsoft.com/en-us/dotnet/api/system.serializableattribute?view=netcore-3.0)

Thanks in advance,

Regards,

Wouter

@StephenBonikowsky
Copy link
Member

@wouterroos This is by design in .NET Core. There were quite a few changes around exceptions between the full framework and core this being one of them. A FaultException should not need to be serialized. Are you doing some custom work where you are doing something like that? If so could you describe your scenario?

@wouterroos
Copy link
Author

@StephenBonikowsky Our repro-case is using WCF including FaultException in Service Fabric and propagate FaultExceptions between services using remoting (which uses binary serialization) and some of these services being ported to NetCore. This works fine when using the full .NET System.ServiceModel but obviously won't using System.ServiceModel.Primitives due to the missing [Serializable] attribute. I don't know if this is a valid use-case from your perspective since there is indeed some 'custom work' involved, but it might highlight a type of 'breaking change' that is introduced by leaving out the attribute.

@StephenBonikowsky
Copy link
Member

@wouterroos Thanks for the additional info. We'll discuss/investigate this and respond back.

@StephenBonikowsky
Copy link
Member

@wouterroos There's nothing wrong with your scenario and we are ok with taking this change in our next release, this could be in a servicing update of release/3.1 or in 5.0.

.NET Core as a whole has adopted the position of not making exceptions Serializable, I believe this is because they want to be able to make changes to exceptions as they wish without breaking anyone. Since this is a WCF specific exception we don't have any issue making this change.

@StephenBonikowsky StephenBonikowsky added this to the 5.0 milestone Nov 13, 2019
@StephenBonikowsky StephenBonikowsky added the feature request Adding new functionality requiring adding an API to the public contract. label Nov 13, 2019
@wouterroos
Copy link
Author

@StephenBonikowsky Great, thanks. Do you have a timeline on the releases you mentioned so we can plan accordingly?

@StephenBonikowsky
Copy link
Member

.NET Core 3.1 releases in December, it's too late for that but it will be serviced beginning in January. We'll try to get it into the servicing release.

@wouterroos
Copy link
Author

@StephenBonikowsky any update on whether this will make it into the service release?

@StephenBonikowsky
Copy link
Member

@wouterroos We have a number of work items on our plate right now. Our plan is to get this into our 5.0 branch and then port it to the 3.1 servicing branch which has monthly releases. I can't be more specific regarding the timeline right now, we are working on our upcoming schedule and then we'll have a better idea.

@StephenBonikowsky StephenBonikowsky added the vnext consider Feature requests or Bugs to consider taking in vNext. label Feb 7, 2020
@wouterroos
Copy link
Author

@StephenBonikowsky Did you get the change to work on the schedule? We're working towards the next release of our product and this is still a blocking issue. Thanks!

@StephenBonikowsky
Copy link
Member

@wouterroos We've been going through a lot of backlogged issues, we are almost done going through those and will then go through the feature requests to do in 5.0.

@HongGit HongGit removed the vnext consider Feature requests or Bugs to consider taking in vNext. label Mar 13, 2020
@HongGit HongGit added the priority 2 Stack ranked level of priority. P2 label Mar 13, 2020
@mconnew
Copy link
Member

mconnew commented May 1, 2020

The fix for this issue is now committed.

@mconnew mconnew closed this as completed May 1, 2020
@wouterroos
Copy link
Author

@mconnew @StephenBonikowsky Thanks! Do you have an ETA on when this will make it into a service release for 3.1?

@StephenBonikowsky
Copy link
Member

@HongGit @mconnew
Have you considered this fix for 3.1?

@wouterroos
Copy link
Author

Hi,

Any update on this?

@wouterroos
Copy link
Author

@StephenBonikowsky @mconnew @HongGit I haven't heard from you regarding this fix on netcore3.1, does this mean this is no longer being considered?

Thanks.

@mconnew
Copy link
Member

mconnew commented Sep 17, 2020

@wouterroos, I'm sorry I missed your message. We don't increase the platform requirements with each of our releases so you can always use the latest WCF packages on all supported .NET Core release. Our latest preview package will still work as far back as .NET Core 2.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Adding new functionality requiring adding an API to the public contract. priority 2 Stack ranked level of priority. P2
Projects
None yet
Development

No branches or pull requests

5 participants