Skip to content

Conversation

@lassetyr
Copy link
Contributor

Adds handling of JsonParseException - now returns a 400 Bad Request.

This typically occurs when GraphQL-requests contain malformed JSON.

@lassetyr lassetyr requested a review from a team as a code owner May 16, 2023 08:34
@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (acd912d) 64.79% compared to head (aabf4eb) 64.80%.

Additional details and impacted files
@@            Coverage Diff             @@
##             dev-2.x    #5121   +/-   ##
==========================================
  Coverage      64.79%   64.80%           
- Complexity     14160    14164    +4     
==========================================
  Files           1733     1733           
  Lines          67487    67498   +11     
  Branches        7221     7223    +2     
==========================================
+ Hits           43729    43739   +10     
  Misses         21330    21330           
- Partials        2428     2429    +1     
Impacted Files Coverage Δ
...opentripplanner/api/common/OTPExceptionMapper.java 0.00% <0.00%> (ø)
...pplanner/graph_builder/module/osm/OsmDatabase.java 78.57% <0.00%> (-0.77%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@t2gran t2gran added this to the 2.4 (next release) milestone May 16, 2023
Copy link
Contributor

@vpaturet vpaturet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with:
{ "query" : "{stopPlace(id:"NSR:StopPlace:17099"){id name estimatedCalls(timeRange:72100 numberOfDepartures:10){realtime aimedArrivalTime aimedDepartureTime expectedArrivalTime expectedDepartureTime destinationDisplay{frontText}}}}" }

I got a warning in the log:
WARN (GraphQL.java:493) Query did not parse : '{ "query" : "{stopPlace(id:"NSR:StopPlace:17099"){id name estimatedCalls(timeRange:72100 numberOfDepartures:10){realtime aimedArrivalTime aimedDepartureTime expectedArrivalTime expectedDepartureTime destinationDisplay{frontText}}}}" }'

but the return code is still 200 with the following GraphQL response:

{
  "errors": [
    {
      "message": "Invalid syntax with offending token '\"query\"' at line 1 column 3",
      "locations": [
        {
          "line": 1,
          "column": 3
        }
      ],
      "extensions": {
        "classification": "InvalidSyntax"
      }
    }
  ]
}

@lassetyr
Copy link
Contributor Author

JsonParseException is thrown when Content-Type-header is set to application/json

@vpaturet
Copy link
Contributor

vpaturet commented May 16, 2023

Ok, so this PR covers the case when a request is sent with the content type "application/json" and the content is invalid JSON. In this case the client gets an HTTP 400 status.
If the content type is "application/graphql" and the content is invalid GraphQL, the client gets an HTTP 200 status with error nodes in the response.

@lassetyr lassetyr merged commit e92fbf9 into opentripplanner:dev-2.x May 16, 2023
@lassetyr lassetyr deleted the handle_json_parse_exception branch May 16, 2023 13:39
t2gran pushed a commit that referenced this pull request May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants