fix: $resultFormat=CSV and GeoJSON crash server with 500, parser token missing#184
Open
Vishmayraj wants to merge 1 commit into
Open
fix: $resultFormat=CSV and GeoJSON crash server with 500, parser token missing#184Vishmayraj wants to merge 1 commit into
Vishmayraj wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While working on the STA to STAC metadata connector, I needed to fetch observations in CSV format to test the pipeline. Every request with
$resultFormat=CSVwas returning 500, so I pulled the Docker logs and traced the error.The root cause was in
lexer.pywhere theRESULT_FORMAT_VALUEtoken pattern only matcheddataArray. So when the parser sawCSV, it tokenized it as anEXPAND_IDENTIFIERand threw an exception. That exception had no dedicated handler inread.py, so it fell through to the genericexcept Exceptionblock and returned 500.Changes
lexer.py=> extend the pattern to cover all three spec-defined values:read.py=> wrapconvert_queryin its own try/except returning 400. Parser exceptions are always caused by malformed client input, not server faults, so 500 was the wrong code here regardless.Note
While tracing this I found that CSV and GeoJSON serialization is not actually implemented. The parser scaffolding exists,
ResultFormatNode,parse_result_format, and the entity validation inconvert_queryare all there, butresult_formatis never passed through to the return dict or the streaming layer inread.py, so the response is always JSON regardless of what$resultFormatis set to. OnlydataArrayappears to have any downstream handling.This PR only fixes the 500 and the wrong error code. The actual serialization is a separate feature that needs to be built. I am opening a separate issue for that.
References
OGC SensorThings API Part 1, Section 9.3.4 => valid
$resultFormatvalues aredataArray,CSV, andGeoJSON.