-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Azure Search .NET SDK throws InvalidOperationException with SerializationBinder #3441
Comments
@NhanNTWPV please add a reference to the latest available Microsoft.Rest.ClientRuntime.Azure in your project. |
Possibly related: #2552 @NhanNTWPV Can you confirm which versions of the following NuGet packages you are using in your project?
|
@brjohnstmsft Confirm working with Microsoft.Rest.ClientRuntime.Azure 3.3.8 & Newtonsoft.Json 10.0.2. Can you make them default dependencies? |
@NhanNTWPV Thanks for the confirmation. We're working on it. I'll keep this issue open to track this work. |
Any update on this? I have a similar exception using
|
@msfcolombo I can't speak for Azure DNS, but we haven't been able to ship an updated version of the Azure Search SDK yet. Have you tried using Microsoft.Rest.ClientRuntime.Azure 3.3.8 or higher with Newtonsoft.Json 10.0.2? That combination seems to work for others. |
Thanks. I upgraded to Microsoft.Rest.ClientRuntime.Azure 3.3.10 and that fixed the issue. This is covered at the already-closed #2552, so I think you can close this as well. |
The fix for this issue shipped with version 3.0.5 to NuGet. |
@brjohnstmsft Something isn't quite right. I've just upgraded to 3.0.5, with Newtonsoft.Json Newtonsoft.Json (10.0.3) and Microsoft.Rest.ClientRuntime.Azure (3.3.10) but am still seeing this exception. |
OK I think you can ignore me - this is due (I think) to an old version of Newtonsoft.Json being picked up in my script! |
OK, I might need some pointers here: I'm seeing the following stack trace when my script goes pop:
I've looked at the old version of the Azure Search SDK and indeed there was a method called Any ideas? |
@isaacabraham Thanks for reporting this. I’m not sure what’s going on. We’ll investigate. Re-opening this issue. |
@isaacabraham I was able to repro the error you're seeing, but only if I explicitly reference Newtonsoft.Json 11.0.1-beta3 from my test project. It appears that in version 11, JSON.NET is deprecating the In the meantime I'll figure out if it's safe to change the Azure Search SDK to use |
Just confirmed that we can't use |
Thanks for checking this out, I'll see if I can repro this. It might be worth changing the max version of Newtonsoft in the Search package to 10.x if it isn't already to prevent dependency issues. |
@isaacabraham Good point. FYI @shahabhijeet @markcowl We found an incompatibility between JSON.NET 11 beta and |
@isaacabraham I assume you are fine when you use newtonsoft 10.0.3 with Search SDK. @brjohnstmsft an alternative to fix this problem is to not to initialize deprecated properties in your CopySettings function (leave out TypeNameAssemblyFormat, ReferenceResolver, Binder) |
@brjohnstmsft also - is there a reason why the Net452 and NetStandard14 versions have different minimum versions of Newtonsoft JSON (i.e. >= 6.0.8 vs >= 9.0.1) ? |
@brjohnstmsft I think I've traced the cause of my particular issue - it's because my script is (for some as-yet unknown reason) implicitly picking up the Newtonsoft.Json assembly inside 6.0.8 does seem to actually contain a |
@isaacabraham currently Azure PowerShell need newtonsoft 6.x.x for .NET 452 target Fx, but all .NET Core Azure SDK are supporting 9.x.x, hence you see two separate minimum versions. |
@shahabhijeet I don't understand - what does Powershell have to do with the minimum version of the Newtonsoft JSON. Are you suggesting that there's actually the possibility of different behaviour between 452 and Core simply because of PowerShell? |
@shahabhijeet FYI, @isaacabraham is the one you should be talking to, not @NhanNTWPV :) @isaacabraham Wow, I didn't realize JSON.NET 6.0.8 doesn't even have the Regarding PowerShell, I believe @shahabhijeet is referring to the Azure PowerShell CmdLets, which depend on various Azure SDKs (excluding Azure Search), which depend on |
@brjohnstmsft Well, it has "a" Binder property - but a different one :-) Regarding the different versions of Newtonsoft across different TFMs - this strikes me at first glance as as a disaster waiting to happen. Picture situations like: you build some code and run it on the net452 TFM. You then change over to the netstandard TFM, and suddenly you've now silently upgraded to a different version of Newtonsoft JSON. You run the same code but now you're getting different behaviour at runtime. Do you know? Will you ever know? Maybe. Maybe not... |
@isaacabraham I agree this is a terrible state of affairs. @shahabhijeet and I are having an offline discussion about possible solutions. |
I had same issue with .Net core. It solved just after degrading newtonsoft.Json to version 10.2. I guess with any of 10 versions it should work |
@isaacabraham I have a PR out that includes the fix for this issue. First we'll fix it in the preview SDK (5.x-preview), then in the stable version of that (5.x, which I'm working on and should release in the coming weeks). I wasn't planning to hotfix it in 3.0. Please let me know if that's a blocker for you. |
@isaacabraham Azure PowerShell consumes Azure SDKs that are targeting .NET 4.5.2 and there are quite few other modules that are still stuck at newtonsoft json 6.0.8, so we are steadily upgrading each of those dependencies to newtonsoftjson 10.x.x until then all SDKs that are targeted .NET 4.5.2 have their minimum version dependency to newtonsoft json 6.x.x |
@shahabhijeet understood. It's just not a good place to be - essentially this breaks the contract for the Search service between different TFMs (and therefore for all .NET developers) for the same of PowerShell. I'm still not sure where the dependency is because this is a decision for Azure Search to make - are you suggesting that there's a huge number of devs using Azure Search from PowerShell compared to e.g. C#? |
@isaacabraham All (or nearly all) Azure .NET SDKs, including Azure Search, rely on the |
@brjohnstmsft ah, that makes more sense - thanks for explaining. So theoretically a new version of ClientRuntime could be made to "fix" this. At any rate the fix you've done should solve this issue anyway :) |
It took longer than I thought, but we finally shipped a new SDK version with a hopefully comprehensive fix for the JSON.NET compatibility issues. It's now available on NuGet. Closing this issue, hopefully for the last time. Thanks for your patience. |
@brjohnstmsft how did you fix this problem? |
@RSuter The later manifestation of the issue was specific to custom code in the Azure Search SDK that copies properties between Line 92 in 68a092d
|
@brjohnstmsft Thanks for the link, we have the exact same problem: https://github.com/RSuter/NSwag/blob/master/src/NSwag.AspNet.WebApi/JsonExceptionFilterAttribute.cs#L106 It would be great if this is part of Newtonsoft.Json (to avoid maintaining this list). Or is there another way to create a serializer settings copy? /cc @JamesNK |
An exception thrown when entering operations with index (Exists, Create, Delete...). Encountered with .NET Core SDK 1.0.4 on Mac OSX 10.12.4
The text was updated successfully, but these errors were encountered: