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

"crs is null" runtime error when handling a valid empty GeoServer response #4438

Closed
esonica opened this issue Oct 17, 2016 · 2 comments
Closed

Comments

@esonica
Copy link

esonica commented Oct 17, 2016

When querying a geoserver that returns an empty collection, the following valid JSON is returned:

{"type":"FeatureCollection","totalFeatures":0,"features":[],"crs":null}

As per the GeoJSON Spec, the CRS value can be a JSON null as valid response :
http://geojson.org/geojson-spec.html#coordinate-reference-system-objects

Currently the Cesium.GeoJsonDataSource.load() function does not handle a null CRS value gracefully, throwing a runtime error.

https://github.com/AnalyticalGraphicsInc/cesium/blob/5a4b5ead78bcb6671f7a55a0cb0847d01e95383c/Source/DataSources/GeoJsonDataSource.js#L873

I don't think a runtime error is applicable when the geoJSON returned is technically valid. Can we find a better way of handling this case?

@esonica esonica changed the title "crs is null" runtime error when handling a valid empty GeoServer request "crs is null" runtime error when handling a valid empty GeoServer response Oct 17, 2016
@mramato
Copy link
Contributor

mramato commented Oct 18, 2016

We could probably special-case an empty feature collection and ignore a crs value of null in this case. But empty data is the only possible use case where a crs of null "makes sense" (though why you would include it at all, I have no idea.). Do you have any other suggestions on how null crs should be handled?

Just an FYI, the official GeoJSON spec is now https://tools.ietf.org/html/rfc7946 and has actually removed the crs property altogether.

mramato added a commit that referenced this issue Oct 19, 2016
Fixes #4438

Also updated changes from #4452, since I forgot to do that originally.
@mramato
Copy link
Contributor

mramato commented Oct 19, 2016

I fixed this with #4456 and it will go out with the next release of Cesium on November 1st.

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

No branches or pull requests

3 participants