Skip to content

Unity / Mono - ClientCertificates NotImplementedException #324

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

Closed
dwelch2344 opened this issue Jan 31, 2021 · 10 comments · Fixed by #325
Closed

Unity / Mono - ClientCertificates NotImplementedException #324

dwelch2344 opened this issue Jan 31, 2021 · 10 comments · Fixed by #325

Comments

@dwelch2344
Copy link
Contributor

Hey @rose-a, new to .NET / Unity but I think I'm running into the #318 ClientWebSocket issue still with 3.2.1 - same scenario as OP but on Mac. The difference I'm seeing is that I'm getting a System.NotImplementedException instead of a PlatformNotSupportedException so the try / catch doesn't do anything.

I can trap the Exception to print it out, and it ends up looking like this:

Subscription Exception Handler - System.NotImplementedException: The method or operation is not implemented.
  at System.Net.Http.HttpClientHandler.get_ClientCertificates () [0x00000] in <7ebf3529ba0e4558a5fa1bc982aa8605>:0 
  at GraphQL.Client.Http.Websocket.GraphQLHttpWebSocket.InitializeWebSocket () [0x000b5] in <a79a8e26596a4659b989a8e130b9b057>:0 
  at GraphQL.Client.Http.Websocket.GraphQLHttpWebSocket+<>c__DisplayClass27_0`1+<<CreateSubscriptionStream>b__5>d[TResponse].MoveNext () [0x00148] in <a79a8e26596a4659b989a8e130b9b057>:0 
UnityEngine.Debug:Log(Object)
<>c:<DoIt>b__4_0(Exception) (at Assets/Api.cs:55)
GraphQL.Client.Http.Websocket.<>c__DisplayClass27_0`1:<CreateSubscriptionStream>b__3(Exception)
System.Reactive.BasicProducer`1:Subscribe(IObserver`1)
GraphQL.Client.Http.Websocket.<<CreateSubscriptionStream>b__5>d:MoveNext()
System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1:Start(<<CreateSubscriptionStream>b__5>d&)
GraphQL.Client.Http.Websocket.<>c__DisplayClass27_0`1:<CreateSubscriptionStream>b__5(IObserver`1)
System.ObservableExtensions:Subscribe(IObservable`1, Action`1)
Api:DoIt() (at Assets/Api.cs:59)
CharacterInventory:TestEquip() (at Assets/CharacterInventory.cs:38)
UnityEngine.EventSystems.EventSystem:Update() (at /Applications/Unity/Hub/Editor/2019.4.18f1/Unity.app/Contents/Resources/PackageManager/BuiltInPackages/com.unity.ugui/Runtime/EventSystem/EventSystem.cs:377)

Is it possible that the certificate library that I've got configured is throwing a different exception than other runtimes or something? Would it be safe to catch both exceptions?

Any help would be appreciated :)

From the Java / Node world by trade, so excuse my sloppy code. Included for reference tho ;)

public class Api { 
    private GraphQLHttpClient client;




    public class Response
    {
        public Post postAdded { get; set; }
    }


    public class Post
    {
        public string author { get; set; }
        public string comment { get; set; }
    }

    public void Start()
    {
        client = new GraphQLHttpClient("http://localhost:4000/graphql", new NewtonsoftJsonSerializer());
        
    }

    public bool DoIt()
    {
        Debug.Log("About to start?");
        var query = new GraphQL.GraphQLRequest
        {
            Query = @"subscription {
                postAdded{
                author
                comment
                }
            }"
        };

            //IObservable<GraphQL.GraphQLResponse<Response>> subscriptionStream = client.CreateSubscriptionStream<Response>(query);
            IObservable<GraphQL.GraphQLResponse<Response>> subscriptionStream = client.CreateSubscriptionStream<Response>(query, ex =>
            {
                Debug.Log("Failed?!? " + ex);
                throw ex;
            });

            var subscription = subscriptionStream.Subscribe(response =>{
                Debug.Log("Got a response");
                Response thing = response.Data;
                Debug.LogFormat("{0} by {1}", thing.postAdded.comment, thing.postAdded.comment);
            });

            Debug.Log("Opened? " + subscription);
    }
}
            
@dwelch2344 dwelch2344 changed the title Unity / Mono Unity / Mono - ClientCertificates NotImplementedException Jan 31, 2021
@dwelch2344
Copy link
Contributor Author

For reference, I believe this might be related - mono/mono#8644

@dwelch2344
Copy link
Contributor Author

Sweet, was just able to confirm the general idea is right. Completely new to Visual Studio IDE, but was able to clone the repo and build locally to resolve my issue. Not sure how to make VS happy with the missing ClientCertificates functions (and it's late so I'm lazy) but I did comment out the entire directive block and rebuilt the lib + my game with success.

Essentially, cutting the following made my local scenario work. Clearly not a good plan for the future, but would adding the additional NotImplemented catch blocks be a problem here?

#if NETFRAMEWORK
// fix websocket not supported on win 7 using
// https://github.com/PingmanTools/System.Net.WebSockets.Client.Managed
_clientWebSocket = SystemClientWebSocket.CreateClientWebSocket();
switch (_clientWebSocket) {
case ClientWebSocket nativeWebSocket:
nativeWebSocket.Options.AddSubProtocol("graphql-ws");
nativeWebSocket.Options.ClientCertificates = ((HttpClientHandler)Options.HttpMessageHandler).ClientCertificates;
nativeWebSocket.Options.UseDefaultCredentials = ((HttpClientHandler)Options.HttpMessageHandler).UseDefaultCredentials;
Options.ConfigureWebsocketOptions(nativeWebSocket.Options);
break;
case System.Net.WebSockets.Managed.ClientWebSocket managedWebSocket:
managedWebSocket.Options.AddSubProtocol("graphql-ws");
managedWebSocket.Options.ClientCertificates = ((HttpClientHandler)Options.HttpMessageHandler).ClientCertificates;
managedWebSocket.Options.UseDefaultCredentials = ((HttpClientHandler)Options.HttpMessageHandler).UseDefaultCredentials;
break;
default:
throw new NotSupportedException($"unknown websocket type {_clientWebSocket.GetType().Name}");
}
#else
_clientWebSocket = new ClientWebSocket();
_clientWebSocket.Options.AddSubProtocol("graphql-ws");
// the following properties are not supported in Blazor WebAssembly and throw a PlatformNotSupportedException error when accessed
try
{
_clientWebSocket.Options.ClientCertificates = ((HttpClientHandler)Options.HttpMessageHandler).ClientCertificates;
}
catch (PlatformNotSupportedException)
{
Debug.WriteLine("property 'ClientWebSocketOptions.ClientCertificates' not supported by current platform");
}
try
{
_clientWebSocket.Options.UseDefaultCredentials = ((HttpClientHandler)Options.HttpMessageHandler).UseDefaultCredentials;
}
catch (PlatformNotSupportedException)
{
Debug.WriteLine("Property 'ClientWebSocketOptions.UseDefaultCredentials' not supported by current platform");
}
Options.ConfigureWebsocketOptions(_clientWebSocket.Options);
#endif

@rose-a
Copy link
Collaborator

rose-a commented Jan 31, 2021

Hi @dwelch2344, thanks for raising this issue.

I think it'd be fine to handle NotImplementedException the same way as PlatformNotSupportedException in this case (in a way "PlattformNotSupported" implicates "NotImplemented"). @sungam3r any thoughts?

I'd welcome a PR, I'm quite occupied at the moment.

@sungam3r
Copy link
Member

Right.

@dwelch2344
Copy link
Contributor Author

Sweet - on it now!

dwelch2344 added a commit to dwelch2344/graphql-client that referenced this issue Jan 31, 2021
@dwelch2344
Copy link
Contributor Author

@rose-a @sungam3r PR opened, let me know if you'd like adjustments or I should adjust anything else. First C# PR so definitely new territory :) thanks!

@dwelch2344
Copy link
Contributor Author

@rose-a @sungam3r Looks like the PR was approved but a test failed. Was that related to my changes? Anything I can do to help out? Thanks in advance :)

@sungam3r
Copy link
Member

sungam3r commented Feb 9, 2021

I am not familiar with the tests of this project. Ping @rose-a

@dwelch2344
Copy link
Contributor Author

Circling back on this. @rose-a thoughts? Anything I can do to help this along?

@rose-a
Copy link
Collaborator

rose-a commented Feb 23, 2021

Hi, sorry for not responding... The tests currently have a stability problem (see #161), I don't think the failing test is due to your changes.

I'll try to merge and release this later today... sorry again for the delay!

rose-a added a commit that referenced this issue Mar 2, 2021
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 a pull request may close this issue.

3 participants