Cannot stringify invalid message; isMessage validation for execution result is too strict about graphql-ws HOT 10 CLOSED

There, you can now pass whatever you want for the payload. 😄

from graphql-ws.

BTW, the client now fails immediately for some fatal close event codes. This should improve the DX by bringing a problem up sooner.

from graphql-ws.

Nothing should be failing silently. Are you sure the socket is not closed with the error in the close event reason? Keep in mind that the client will retry a few times if the connection gets closed (you might not see the error immediately).

If its indeed failing silently, please make a test or a repro and I will be more than happy to look at it and bring up a fix! (Please make an additional issue)

Furthermore, the server validates messages because they should follow the Protocol's shapes. I'd say the validation for the next message is very forgiving - it requires the data or the errors field to be an object in the payload:

It does make sense to support whatever for the data field - fix is coming!

from graphql-ws.

The example above shows such a query, with the schema and Query Object type 😅

You are right, the client connection gets closed and it will retry connecting. However, since this is a error in general related to the server, I would actually also expect to see that issue logged to the console.

The close reason for socket is also not detailed enough imho. like is the message sent by the client invalid or the one sent back and what exactly is invalid? For a nice DX A more detailed message could be returned.

I would propose to not explicitly check whether payload has a data or errors property. That seems to be an implementation detail of execute to me

from graphql-ws.

You can hook up onto the closed event for the client and log all socket closes.

I check just the basics, the simplest form of graphql-js's ExecuteResult (any data or object errors). The message has a super simple form, so it should be rather easy to detect why the message is malformed - hence vague error.

I plan on making the client retry connections on some errors only - classify the fatal ones and report them immediately.

Furthermore, all of server-side error handling issues will go away as soon as #64 lands. You will have full control of error handling and reporting then. Once this lands, tracking if the malformed issue originates from the back or front should be easy. Will keep your insights in mind though!

from graphql-ws.

defer and stream AsyncIterables returned from execute will emit a final { hasNext: false } execution result. See src/execution/tests/stream-test.js here: graphql/graphql-js@dec5554#diff-a370dc23e95e90e2b04fe67a3a933e6eb09296c64b4eae7509cd3ae93edaf2ae

from graphql-ws.

Oh! Well, thats enough for me. I'll just make it so that the validation makes sure that the payload is present. You can pass whatever you want.

from graphql-ws.

Awesome 👏 Thanks for considering this!

from graphql-ws.

Thank you for bringing it up!

from graphql-ws.

🎉 This issue has been resolved in version 1.13.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

from graphql-ws.