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

Make routeLineclipping robust to malformed gtfs #36

Merged
merged 2 commits into from
Dec 14, 2024

Conversation

arindam1993
Copy link
Contributor

Added and null check and wrapped it in a try..catch 🤷🏽

@arindam1993 arindam1993 requested a review from graue December 13, 2024 00:45
@arindam1993
Copy link
Contributor Author

Deployed onto staging, seems to be working well!

Comment on lines 41 to 43
const shapeIdsForRoute = stopTripShapeLookup[routeId];

if (shapeIdsForRoute) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this the only thing that could be undefined, therefore doesn't this check make the catch-all around everything unnecessary?

Copy link
Contributor Author

@arindam1993 arindam1993 Dec 13, 2024

Choose a reason for hiding this comment

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

lineSlice can throw if any of the input is malformed in any way. A linestring that's a single point for example.
Since this part of the code deals with data parsed from the gtfs files which don't have a strict schema, I like having this safety net.
A much more elegant solution would be doing schema validation on the ingested dictionaries, but eh, thats way too much work for some data that needs to only be used in one function.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put the try/catch around just the lineSlice call then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, I wrapped all the turf punctions though coz all of them have some validation

@arindam1993 arindam1993 merged commit b507488 into main Dec 14, 2024
3 checks passed
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.

2 participants