yelp-fusion: Setting PRICE, LATITUDE, LONGITUDE parameters the wrong way returns Server Code 200 w/ no data.

For 3:

the error message not being parseable as valid json

I can just make it valid JSON with the quotes escaped… but on further thought I’m now leaning towards just changing the outputs to be more string-like.

Instead of:

{
	"errors": [
		{
			"message": "{'error': {'code': 'LOCATION_MISSING', 'description': 'You must specify either a location or a latitude and longitude to search.'}}",
			"locations": [
				{
					"line": 2,
					"column": 3
				}
			]
		}
	],
	"data": {...data goes here...}
}

The output would be more like:

{
	"errors": [
		{
			"message": "LOCATION_MISSING: You must specify either a location or a latitude and longitude to search.",
			"locations": [
				{
					"line": 2,
					"column": 3
				}
			]
		}
	],
	"data": {...data goes here...}
}

This feels a bit more in-line with other graphql implementations out there. We’d have the error string format be “CODE: DESCRIPTION”.

latitude=0.0 causes server error

This fix is now live also; I checked that:

{
  search(term: "Restaurants", latitude: 0, longitude: 180, radius: 10000, limit: 35) {..OMITTED..}
}

no longer errors out.

@coolboiime OK I hear you… so you’d prefer just the message as-is, but jumped to json. How about:

{
	"errors": [
		{
			"message": "{\"error\": {\"code\": \"BUSINESS_NOT_FOUND\", \"description\": \"The requested business could not be found.\", \"id\": \"yelp-san-franciscoo\"}}",
			"locations": [
				{
					"line": 2,
					"column": 3
				}
			]
		}
	],
	"data": {
		"business": null
	}
}

I’m still leaning towards the more human-readable message form of CODE: DESCRIPTION to avoid having the escaped json in the message. It would be nicer to just add the various fields alongside ‘message’ and ‘locations’ but would violate the formatting errors section of the GraphQL spec. A nice alternative would be to put our error code beneath ‘extensions’ and I’d like to do that as a future enhancement.

If we committed to following that message format then a user could potentially get the code by splitting the message string on ‘:’.

We’ll get the ‘path’ param in error messages once the next version of graphql-core is released, which will also help with handling/understanding errors.

@coolboiime looks like there are 3 bugs in our systems you’ve identified here:

1. price filter with spaces causes server error
2. latitude=0.0 causes server error
3. the error message not being parseable as valid json

I’m deploying the fix for 1 right now, should be live in a couple minutes. 2 is in code review, and I’ll fix 3 later today or tomorrow.

Thank you so much again for the great bug report!

One other thing - currently the graphql api tries to return a response with a 200 status code in all cases except for authentication errors. In the body of the graphql api response there is an errors list like you see in the output you posted; in this errors list we will dump the errors encountered while executing your graphql query. Whenever possible we will try to return as much of the query result as possible, even if there are errors encountered along the way. We are somewhat inspired by github’s graphql API in how we think about errors.

As you’ve seen, since Yelp’s GraphQL API is still young and rough around the edges there are plenty of bugs to be discovered and fixed. In addition, if you’re ever unsure whether some behavior is a bug or expected (especially if our docs don’t specify) then please file an issue so we can fix the API or fix the docs.

@coolboiime thanks for the detailed bug report! I’ll file a ticket internally and start looking into this.