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

Improve handling of various non-serializable Exception situations #602

Merged
merged 6 commits into from
Nov 3, 2020

Conversation

AArnott
Copy link
Member

@AArnott AArnott commented Nov 3, 2020

There were a few issues:

  1. We didn't gracefully handle exception types that were missing the [Serializable] attribute. Now we log a warning and just write out CommonErrorData.
  2. We didn't gracefully handle exception types that were missing the deserializing constructor. Now we log a warning and deserialize the nearest base type that does conform to the serializable pattern.
  3. We weren't even serializing thrown exceptions to JSON ourselves (newtonsoft.json was doing it) which means it wasn't treated as 'user data'. Now we serialize thrown exceptions with our own converter as user data.
  4. Exceptions weren't serialized to include their assembly name, so unless the assembly was already loaded, we wouldn't be able to activate it. Now we artificially add AssemblyName as a property of the exception.
  5. Even if exception's assembly were already loaded, unless it was loaded in the From load context it still wouldn't be found. That's still the case with this change, but if the assembly name is specified we'll load it into the From context automatically, assuming the CLR can find it.
  6. Even if the assembly was loaded in the From context, if the assembly version of the transmitting side exceeded the assembly version on the receiving side, it wouldn't load properly. Now we fallback to allow an assembly version mismatch (provided the name and public key token still match). We further fallback to any (loaded) assembly that defines a type with a matching full name.

Fixes #588
See also DevDiv-1240018, #468

@AArnott AArnott added this to the v2.7 milestone Nov 3, 2020
@AArnott AArnott self-assigned this Nov 3, 2020
@AArnott AArnott requested a review from ankitvarmait November 3, 2020 13:43
@AArnott AArnott merged commit 46701b9 into microsoft:master Nov 3, 2020
@AArnott AArnott deleted the fix588 branch November 3, 2020 19:27
osexpert pushed a commit to osexpert/vs-streamjsonrpc that referenced this pull request Jul 29, 2023
Improve handling of various non-serializable Exception situations
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.

Exceptions without [Serializable] fail to serialize/deserialize properly
2 participants