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

Subscription [GraphQL] errors not being caught #92

Closed
chelliwell opened this issue Jan 7, 2020 · 9 comments
Closed

Subscription [GraphQL] errors not being caught #92

chelliwell opened this issue Jan 7, 2020 · 9 comments
Labels
Milestone

Comments

@chelliwell
Copy link
Contributor

I've encountered a situation where my server is generating a message containing errors:

{
	"type": "data",
	"id": "eb40cfadcffe475c8cae314bab84015c",
	"payload": {
		"errors": [{
			"message": "Int cannot represent non-integer value: 0.46957214897012906",
			"locations": [{
				"line": 1,
				"column": 196
			}],
			"path": ["dfRdr", "record", "emitterIdConfidence"],
			"extensions": {
				"code": "INTERNAL_SERVER_ERROR"
			}
		}],
		"data": { ...<private project info>..... }
	}
}

It looks, from GraphQLSubscriptionOperation.OperationSource_RecievePayload(), like this should fire the ErrorRecieved event handler - but this doesn't seem to be happening. (I have it hooked in my app).
I've tried to step into the source (hopefully the correct git version for the NuGet package I'm using...):

        private void OperationSource_RecievePayload(object sender, PayloadEventArgs e)
        {
            if (_isStopped)
                return;

            // Get GraphQLResult
            var result = e.Payload.ToObject<GraphQLDataResult<JObject>>();

            // Add final result
            var finalResult = new GraphQLDataResult<T>()
            {
                Errors = result.Errors
            };

            // Deserilize data
            if (result.Data != null)
            {
                var data = deserialization.DeserializeResult<T>(result.Data, selectionSet);
                finalResult.Data = data;
            }

            // Send event
            if (result.ContainsErrors)
            {
                ErrorRecieved?.Invoke(this, new GraphQLDataReceivedEventArg<T>(finalResult));
            }
            else
            {
                DataRecieved?.Invoke(this, new GraphQLDataReceivedEventArg<T>(finalResult));
            }
        }

result does contain errors, but "if (result.ContainsErrors)" is never reached - from DerializeResult it jumps to SendMessageAsync().
Perhaps because the errored field ('emitterIdConfidence') has a value of null...?

[Note: the error is due to incorrect coding of the server's schema ('emitterIdConfidence' should be declared as Float) - but it would be nice to have the error appear somewhere]

@chelliwell
Copy link
Contributor Author

If I declare 'emitterIdConfidence' as nullable in the client schema (which it isn't meant to be), then I do get the ErrorRecieved event.

@sahb1239 sahb1239 added the bug label Jan 7, 2020
@sahb1239
Copy link
Owner

sahb1239 commented Jan 7, 2020

Will you try version 2.1.2-beta0001 on the AppVeyor NuGet feed?

It can be found here:
https://ci.appveyor.com/nuget/sahb-graphqlclient-jry5sxi8qeq7

@chelliwell
Copy link
Contributor Author

I should be able to try that tomorrow, I have appveyor set up as a NuGet feed for my solution.
However I'm currently pulling '2.2.0-simplifyhttpmeth0001' - not sure what I might lose by 2.2.0 -> 2.1.2? Hopefully at least be able to try out the beta0001 mod though for this particular issue though - will let you know asap. Many thanks.

@chelliwell
Copy link
Contributor Author

Unfortunately, I think - with the change being based on 2.1.2 - I'm now missing #77 so I can't actually subscribe at all

@chelliwell
Copy link
Contributor Author

I've been adding a new subscription (with no enum), which should allow me to check the fix in 2.1.2-beta0001.
The change in hotfix/92 is what I imagined the fix might be but, I think, it doesn't seem to resolve the issue - if the bad field is non-nullable. [I presume this fix is in 2.1.2-beta0001?]

@sahb1239
Copy link
Owner

I have merged the fix into develop. Would you try version SAHB.GraphQL.Client.2.2.0-alpha0085.nupkg?

@chelliwell
Copy link
Contributor Author

Yep, that seems to be catching the error ok.

By the way - do you have a planned timeline for a 2.2.0 release? I'd be happy, and keen, to continue working with further enhancements, but it would also be good to re-baseline my project to a formal release in the meantime.

@sahb1239
Copy link
Owner

I have now created a 2.2.0 release. It should contain the fix.

@sahb1239 sahb1239 added this to the 2.2.0 milestone Feb 11, 2020
@chelliwell
Copy link
Contributor Author

Great - thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants