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

fix TripTime NullPointerException #6094

Closed
wants to merge 2 commits into from

Conversation

miklcct
Copy link
Contributor

@miklcct miklcct commented Sep 26, 2024

Summary

Fixes NullPointerException on TripTime. The value is clearly nullable but no null check took place, causing NullPointerException.

Issue

Fixes #6088

Unit tests

No idea yet.

Documentation

None needed

Changelog

Bumping the serialization version id

@miklcct
Copy link
Contributor Author

miklcct commented Sep 26, 2024

I have tested that this does prevent error coming from the API, and my frontend looks good. However, my frontend only uses the content for some logic and it didn't show the contents of affected trips.

Copy link

codecov bot commented Sep 27, 2024

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Please upload report for BASE (dev-2.x@155d19b). Learn more about missing BASE report.
Report is 160 commits behind head on dev-2.x.

Files with missing lines Patch % Lines
...java/org/opentripplanner/model/TripTimeOnDate.java 0.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             dev-2.x    #6094   +/-   ##
==========================================
  Coverage           ?   69.84%           
  Complexity         ?    17454           
==========================================
  Files              ?     1975           
  Lines              ?    74604           
  Branches           ?     7636           
==========================================
  Hits               ?    52107           
  Misses             ?    19846           
  Partials           ?     2651           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@miklcct miklcct marked this pull request as ready for review September 27, 2024 12:14
@miklcct miklcct requested a review from a team as a code owner September 27, 2024 12:14
@optionsome
Copy link
Member

We should probably get an idea what is the original data(?) problem that causes this in the first place. There might be better way to solve this specific problem.

@vpaturet
Copy link
Contributor

The root cause of the issue seems to be a bug in the GraphQL API where the scheduled timetable is used to resolve a real-time updated trip, leading to data inconsistency (to be confirmed and further analyzed, see org.opentripplanner.apis.gtfs.datafetchers.TripImpl.stopTimes())

We discussed the fix proposed in this PR during the developer meeting and agreed that handling the null case would hide a potential bug and propagate inconsistent data down the stack.

In this particular case we suggest addressing the root cause of the bug rather than handling the NPE.

@t2gran t2gran added this to the 2.7 (next release) milestone Oct 16, 2024
@t2gran
Copy link
Member

t2gran commented Oct 16, 2024

We will close this next week unless we get more insight/feedback on this.

@optionsome optionsome closed this Oct 22, 2024
@t2gran t2gran modified the milestones: 2.7 (next release), Rejected Oct 22, 2024
@miklcct miklcct deleted the triptime-npe branch November 8, 2024 16:14
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.

Exception while fetching data. NullPointerException because times is null
4 participants