Financial-Times / biz-ops-client

Safely send and retrieve data from the FT Biz Ops API.
0 stars 1 forks source link

Documentation of graphql error conditions is wrong #45

Open GeoffThorpeFT opened 7 months ago

GeoffThorpeFT commented 7 months ago

The README states:

graphQL.post(query: string, variables?: object, strict?: boolean)
Fetches data from the Biz Ops GraphQL API using a POST request. You should use this if data must always be up-to-date. Resolves to the data returned. **Rejects with a [BadRequest](https://github.com/Financial-Times/biz-ops-client#errors) if the query is invalid.** When "strict mode" is enabled the promise will also be rejected with a [GraphQLError](https://github.com/Financial-Times/biz-ops-client#errors) if a successful response includes any errors.

The above is incorrect as it does not reject when the query has syntax errors.

As stated in the second sentence strict mode rejects when errors are found.

The confusion between these two statements means that all our users who dont use strict only get thrown errors when biz ops itself fails - all the syntax errors are swallowed.

GeoffThorpeFT commented 7 months ago

This is just to fix the documentation.

If we wish to fix the underlying problem - and meet the specifiation laid out in the README - then we need to return syntax errors regardless of the state of strict https://github.com/Financial-Times/biz-ops-client/issues/23

i-like-robots commented 7 months ago

Has this been resolved via #46 ?