Not implemented exceptions in Getters

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Not implemented exceptions in Getters

Kevin Connell
I have run into problems Serializing a hashtable with Json.net and found that some classes (maybe just one?) throws NotImplementedExceptions in a getter which obviously cause havoc for Serialization and other generic reflection routines.  Is throwing a NotImplementedExceptions  an appropriate pattern in a getter?





_______________________________________________
Mono-list maillist  -  [hidden email]
http://lists.ximian.com/mailman/listinfo/mono-list
Reply | Threaded
Open this post in threaded view
|

Re: Not implemented exceptions in Getters

David Curylo
On Friday, July 11, 2014, Kevin Connell <[hidden email]> wrote:
Is throwing a NotImplementedExceptions  an appropriate pattern in a getter?


Not really an appropriate pattern, no. Which classes were these? Part of the mono framework?

"Avoid throwing exceptions from property getters.
Property getters should be simple operations without any preconditions. If a getter might throw an exception, consider redesigning the property to be a method. This recommendation does not apply to indexers. Indexers can throw exceptions because of invalid arguments."


_______________________________________________
Mono-list maillist  -  [hidden email]
http://lists.ximian.com/mailman/listinfo/mono-list
Reply | Threaded
Open this post in threaded view
|

Fwd: Not implemented exceptions in Getters

Kevin Connell


---------- Forwarded message ----------
From: Kevin Connell <[hidden email]>
Date: Fri, Jul 11, 2014 at 8:18 PM
Subject: Re: [Mono-list] Not implemented exceptions in Getters
To: Dave Curylo <[hidden email]>


Here is the stack trace

Newtonsoft.Json.JsonSerializationException: Error getting value from 'IsSecurityCritical' on 'System.Reflection.MonoMethod'. ---> System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.NotImplementedException: The requested feature is not implemented.
  at System.Reflection.MethodBase.get_IsSecurityCritical () [0x00000] in <filename unknown>:0 
  at (wrapper delegate-invoke) <Module>:invoke_bool__this___MethodBase (System.Reflection.MethodBase)
  at System.Reflection.MonoProperty.GetterAdapterFrame[MethodBase,Boolean] (System.Reflection.Getter`2 getter, System.Object obj) [0x00000] in <filename unknown>:0 
  at System.Reflection.MonoProperty.GetValue (System.Object obj, System.Object[] index) [0x00000] in <filename unknown>:0 
  --- End of inner exception stack trace ---
  at System.Reflection.MonoProperty.GetValue (System.Object obj, System.Object[] index) [0x00000] in <filename unknown>:0 
  at Newtonsoft.Json.Utilities.ReflectionUtils.GetMemberValue (System.Reflection.MemberInfo member, System.Object target) [0x00000] in <filename unknown>:0 
  at Newtonsoft.Json.Serialization.ReflectionValueProvider.GetValue (System.Object target) [0x00000] in <filename unknown>:0 
  --- End of inner exception stack trace ---
  at Newtonsoft.Json.Serialization.ReflectionValueProvider.GetValue (System.Object target) [0x00000] in <filename unknown>:0 
  at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.CalculatePropertyValues (Newtonsoft.Json.JsonWriter writer, System.Object value, Newtonsoft.Json.Serialization.JsonContainerContract contract, Newtonsoft.Json.Serialization.JsonProperty member, Newtonsoft.Json.Serialization.JsonProperty property, Newtonsoft.Json.Serialization.JsonContract& memberContract, System.Object& memberValue) [0x00000] in <filename unknown>:0 
  at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeObject (Newtonsoft.Json.JsonWriter writer, System.Object value, Newtonsoft.Json.Serialization.JsonObjectContract contract, Newtonsoft.Json.Serialization.JsonProperty member, Newtonsoft.Json.Serialization.JsonContainerContract collectionContract, Newtonsoft.Json.Serialization.JsonProperty containerProperty) [0x00000] in <filename unknown>:0 
2014-07-11 11:39:27,093 


On Fri, Jul 11, 2014 at 8:07 PM, Dave Curylo <[hidden email]> wrote:
On Friday, July 11, 2014, Kevin Connell <[hidden email]> wrote:
Is throwing a NotImplementedExceptions  an appropriate pattern in a getter?


Not really an appropriate pattern, no. Which classes were these? Part of the mono framework?

"Avoid throwing exceptions from property getters.
Property getters should be simple operations without any preconditions. If a getter might throw an exception, consider redesigning the property to be a method. This recommendation does not apply to indexers. Indexers can throw exceptions because of invalid arguments."




_______________________________________________
Mono-list maillist  -  [hidden email]
http://lists.ximian.com/mailman/listinfo/mono-list
Reply | Threaded
Open this post in threaded view
|

Re: Not implemented exceptions in Getters

Kevin Connell
In reply to this post by Kevin Connell

Works fine with .net framework.  As I mentioned its Json.net serializing a hash table and you can see from the stacktrace its a mono class with the getter that has the throw.

On Jul 12, 2014 9:24 AM, "Edward Ned Harvey (mono)" <[hidden email]> wrote:
> From: [hidden email] [mailto:[hidden email]
> [hidden email]] On Behalf Of Kevin Connell
>
> I have run into problems Serializing a hashtable with Json.net and found that
> some classes (maybe just one?) throws NotImplementedExceptions in a
> getter which obviously cause havoc for Serialization and other generic
> reflection routines.  Is throwing a NotImplementedExceptions  an
> appropriate pattern in a getter?

There are lots of behaviors, appropriate or not, resulting from the fact that somebody at Microsoft says so.

What's the behavior of the same code, when run under .Net?  If MS throws the exception too, then mono is behaving correctly, as the goal of mono is to behave the same as .Net.

If mono is not behaving the same as .Net, please be specific about what class(es) and how to reproduce the problem behavior.

_______________________________________________
Mono-list maillist  -  [hidden email]
http://lists.ximian.com/mailman/listinfo/mono-list
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Not implemented exceptions in Getters

Kevin Connell
In reply to this post by Kevin Connell

So, I should fork mono so I can serialize a hashtable with an industry standard serialization library?

On Jul 12, 2014 9:33 AM, "Edward Ned Harvey (mono)" <[hidden email]> wrote:
> From: [hidden email] [mailto:[hidden email]
> [hidden email]] On Behalf Of Kevin Connell
>
> Newtonsoft.Json.JsonSerializationException: Error getting value from
> 'IsSecurityCritical' on 'System.Reflection.MonoMethod'. --->

I would say, I don't think it makes sense to be serializing System.Reflection in general, and it definitely doesn't make sense to be serializing IsSecurityCritical in particular.  You should probably think about what you're serializing, and consider decorating it with [JsonIgnoreAttribute] or similar.

_______________________________________________
Mono-list maillist  -  [hidden email]
http://lists.ximian.com/mailman/listinfo/mono-list
Reply | Threaded
Open this post in threaded view
|

Re: Not implemented exceptions in Getters

David Curylo
On Saturday, July 12, 2014, Kevin Connell <[hidden email]> wrote:

So, I should fork mono so I can serialize a hashtable with an industry standard serialization library?

Not unless there is a mono bug here to fix, and then a fork, test, and pull request would be needed to fix it. 

I use json.net successfully on mono for a few different projects, although I don't often just serialize hashtables since they can be tricky to deserialize without type information.  Can you provide a code sample to reproduce this?

An exception from a getter is not the behavior people generally expect, so I agree it's poor design that will cause unexpected errors (like yours) when people try to use the class in a typical way, but maybe there is a good reason that it's not implemented on mono.

_______________________________________________
Mono-list maillist  -  [hidden email]
http://lists.ximian.com/mailman/listinfo/mono-list
Reply | Threaded
Open this post in threaded view
|

Re: Not implemented exceptions in Getters

David Curylo
The following code works fine on current GA releases of mono 3.4.0 and Newtonsoft.Json 6.0.3, btw. It's simplistic, but just add whatever else you have in your hashtable that's resulting in an exception and reply with that source.  I'm sure it isn't the hashtable itself that can't be serialized.

using System;
using System.Collections;
using Newtonsoft.Json;

namespace Test {
        public class Ser {
                public static void Main(string [] args) {
                        var ht = new Hashtable();
                        ht.Add("test", 1234);
                        var json = JsonConvert.SerializeObject(ht);
                        Console.WriteLine(json);
                }
        }
}
 

_______________________________________________
Mono-list maillist  -  [hidden email]
http://lists.ximian.com/mailman/listinfo/mono-list
Reply | Threaded
Open this post in threaded view
|

Re: Not implemented exceptions in Getters

Kevin Connell
https://github.com/mono/mono/blob/master/mcs/class/corlib/System.Reflection/FieldInfo.cs

Line 281

Sorry, Not sure on the process for a pull request, but its a getter on a public property with a throw in the main branch,and its the same property that is indicated in the stack trace I sent.  

cc:author





On Sat, Jul 12, 2014 at 4:07 PM, Dave Curylo <[hidden email]> wrote:
The following code works fine on current GA releases of mono 3.4.0 and Newtonsoft.Json 6.0.3, btw. It's simplistic, but just add whatever else you have in your hashtable that's resulting in an exception and reply with that source.  I'm sure it isn't the hashtable itself that can't be serialized.

using System;
using System.Collections;
using Newtonsoft.Json;

namespace Test {
        public class Ser {
                public static void Main(string [] args) {
                        var ht = new Hashtable();
                        ht.Add("test", 1234);
                        var json = JsonConvert.SerializeObject(ht);
                        Console.WriteLine(json);
                }
        }
}
 


_______________________________________________
Mono-list maillist  -  [hidden email]
http://lists.ximian.com/mailman/listinfo/mono-list
Reply | Threaded
Open this post in threaded view
|

Re: Not implemented exceptions in Getters

David Curylo
Git blame shows Marek Safar added those properties.  They are needed for .NET 4.0 compatibility, but since mono does not support code access security, they can't really be supported.  The argument could be made for these to return false since everything running under mono essentially runs as fully trusted and nothing gets special treatment as "security critical" code.  Code access security is not implemented, but then at least these properties could be.

To solve your immediate problem, you could provide a custom IContractResolver and tell it to ignore get-only properties, which would cover these that aren't implemented.  Chances are good if you're serializing types you can't control, you'll eventually need this sort of customization anyway.  See here for an example:


On Saturday, July 12, 2014, Kevin Connell <[hidden email]> wrote:
https://github.com/mono/mono/blob/master/mcs/class/corlib/System.Reflection/FieldInfo.cs

Line 281

Sorry, Not sure on the process for a pull request, but its a getter on a public property with a throw in the main branch,and its the same property that is indicated in the stack trace I sent.  

cc:author


_______________________________________________
Mono-list maillist  -  [hidden email]
http://lists.ximian.com/mailman/listinfo/mono-list
Reply | Threaded
Open this post in threaded view
|

Re: Not implemented exceptions in Getters

Kevin Connell

Ummm.. dont think its a viable option to not serialize get only properties. 

I have been able to work around it but it's just stupid to have had to create a custom serialization routine for string:string in a hashtable.  Hello.

On Jul 12, 2014 9:08 PM, "Dave Curylo" <[hidden email]> wrote:
Git blame shows Marek Safar added those properties.  They are needed for .NET 4.0 compatibility, but since mono does not support code access security, they can't really be supported.  The argument could be made for these to return false since everything running under mono essentially runs as fully trusted and nothing gets special treatment as "security critical" code.  Code access security is not implemented, but then at least these properties could be.

To solve your immediate problem, you could provide a custom IContractResolver and tell it to ignore get-only properties, which would cover these that aren't implemented.  Chances are good if you're serializing types you can't control, you'll eventually need this sort of customization anyway.  See here for an example:


On Saturday, July 12, 2014, Kevin Connell <[hidden email]> wrote:
https://github.com/mono/mono/blob/master/mcs/class/corlib/System.Reflection/FieldInfo.cs

Line 281

Sorry, Not sure on the process for a pull request, but its a getter on a public property with a throw in the main branch,and its the same property that is indicated in the stack trace I sent.  

cc:author


_______________________________________________
Mono-list maillist  -  [hidden email]
http://lists.ximian.com/mailman/listinfo/mono-list
Reply | Threaded
Open this post in threaded view
|

Re: Not implemented exceptions in Getters

David Curylo
Hello, there.  I don't know enough about what you're trying to do to be able to propose a viable option, just trying to help you out, and if you can't set the properties on deserialization, it seems worthless to serialize them.  Anyway, you're clearly not serializing a hashtable of string:string.  Doing so requires no custom serialization and as you pointed out, your stack trace shows the exception serializing FieldInfo.  But if you're writing something that is so tightly coupled to .NET that you're serializing reflection types, then why bother with JSON serialization?

On Saturday, July 12, 2014, Kevin Connell <[hidden email]> wrote:

Ummm.. dont think its a viable option to not serialize get only properties. 

I have been able to work around it but it's just stupid to have had to create a custom serialization routine for string:string in a hashtable.  Hello.

On Jul 12, 2014 9:08 PM, "Dave Curylo" <<a href="javascript:_e(%7B%7D,&#39;cvml&#39;,&#39;curylod@asme.org&#39;);" target="_blank">curylod@...> wrote:
Git blame shows Marek Safar added those properties.  They are needed for .NET 4.0 compatibility, but since mono does not support code access security, they can't really be supported.  The argument could be made for these to return false since everything running under mono essentially runs as fully trusted and nothing gets special treatment as "security critical" code.  Code access security is not implemented, but then at least these properties could be.

To solve your immediate problem, you could provide a custom IContractResolver and tell it to ignore get-only properties, which would cover these that aren't implemented.  Chances are good if you're serializing types you can't control, you'll eventually need this sort of customization anyway.  See here for an example:


On Saturday, July 12, 2014, Kevin Connell <<a href="javascript:_e(%7B%7D,&#39;cvml&#39;,&#39;kevin@connells.net&#39;);" target="_blank">kevin@...> wrote:
https://github.com/mono/mono/blob/master/mcs/class/corlib/System.Reflection/FieldInfo.cs

Line 281

Sorry, Not sure on the process for a pull request, but its a getter on a public property with a throw in the main branch,and its the same property that is indicated in the stack trace I sent.  

cc:author


_______________________________________________
Mono-list maillist  -  [hidden email]
http://lists.ximian.com/mailman/listinfo/mono-list