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

Include the child tag name into 'InvalidChildElement' error. #7

Closed
wants to merge 1 commit into from

Conversation

zudov
Copy link
Contributor

@zudov zudov commented Feb 23, 2018

This makes the error much more useful. Including the actual name of a tag that caused problem makes it possible to figure out what's wrong with the gpx file without having to go through the library code finding out what did it expect.

Full context

I've been trying to parse the gpx track from my Garmin device, but gpx::read errored out with:

invalid child element in track

I opened the file and saw the following:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<gpx xmlns="http://www.topografix.com/GPX/1/1" xmlns:gpxx="http://www.garmin.com/xmlschemas/GpxExtensions/v3" xmlns:gpxtrkx="http://www.garmin.com/xmlschemas/TrackStatsExtension/v1" xmlns:wptx1="http://www.garmin.com/xmlschemas/WaypointExtension/v1" xmlns:gpxtpx="http://www.garmin.com/xmlschemas/TrackPointExtension/v1" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" creator="GPSMAP 64s" version="1.1" xsi:schemaLocation="http://www.topografix.com/GPX/1/1 http://www.topografix.com/GPX/1/1/gpx.xsd http://www.garmin.com/xmlschemas/GpxExtensions/v3 http://www8.garmin.com/xmlschemas/GpxExtensionsv3.xsd http://www.garmin.com/xmlschemas/TrackStatsExtension/v1 http://www8.garmin.com/xmlschemas/TrackStatsExtension.xsd http://www.garmin.com/xmlschemas/WaypointExtension/v1 http://www8.garmin.com/xmlschemas/WaypointExtensionv1.xsd http://www.garmin.com/xmlschemas/TrackPointExtension/v1 http://www.garmin.com/xmlschemas/TrackPointExtensionv1.xsd">
  <metadata>
    <link href="http://www.garmin.com">
      <text>Garmin International</text>
    </link>
    <time>2018-02-22T10:40:38Z</time>
  </metadata>
  <trk>
    <name>2018-02-22 11:40:31</name>
    <extensions>
      <gpxx:TrackExtension>
        <gpxx:DisplayColor>Blue</gpxx:DisplayColor>
      </gpxx:TrackExtension>
      <gpxtrkx:TrackStatsExtension>
        <gpxtrkx:Distance>60986</gpxtrkx:Distance>
        <gpxtrkx:TotalElapsedTime>88210</gpxtrkx:TotalElapsedTime>
        <gpxtrkx:MovingTime>14117</gpxtrkx:MovingTime>
        <gpxtrkx:StoppedTime>4865</gpxtrkx:StoppedTime>
        <gpxtrkx:MovingSpeed>4</gpxtrkx:MovingSpeed>
        <gpxtrkx:MaxSpeed>19</gpxtrkx:MaxSpeed>
        <gpxtrkx:MaxElevation>1334</gpxtrkx:MaxElevation>
        <gpxtrkx:MinElevation>1102</gpxtrkx:MinElevation>
        <gpxtrkx:Ascent>1692</gpxtrkx:Ascent>
        <gpxtrkx:Descent>1660</gpxtrkx:Descent>
        <gpxtrkx:AvgAscentRate>0</gpxtrkx:AvgAscentRate>
        <gpxtrkx:MaxAscentRate>2</gpxtrkx:MaxAscentRate>
        <gpxtrkx:AvgDescentRate>0</gpxtrkx:AvgDescentRate>
        <gpxtrkx:MaxDescentRate>-4</gpxtrkx:MaxDescentRate>
      </gpxtrkx:TrackStatsExtension>
    </extensions>
    <trkseg>
      <trkpt lat="60.4295433965" lon="7.7490838990">
        <ele>1166.69</ele>
        <time>2018-02-21T10:10:28Z</time>
      </trkpt>
      <trkpt lat="60.4295495991" lon="7.7490779478">
        <ele>1166.69</ele>
        <time>2018-02-21T10:10:29Z</time>
      </trkpt>
      <trkpt lat="60.4295322485" lon="7.7490347810">
        <ele>1166.82</ele>
        <time>2018-02-21T10:11:00Z</time>
      </trkpt>
      ...many...more...track...points....
    </trkseg>
  </trk>
</gpx>

It was my first time looking at GPX and my first time using rust-gpx (also the first time playing with Rust), so I couldn't tell right away which part of that XML is considered invalid. However I saw that the xml schema/namespace references some Garmin's extensions to GPX and thought that it must be what rust-gpx doesn't like.

I had a look at src/parser/track.rs and saw that it matches some standard tags, and for any other tag it just returns InvalidChildElement("track") throwing away the actual name of "invalid" element.

I didn't want to keep eyeballing my file, cross-referencing it with the list of tags expected by track.rs. Instead I wanted to actually improve the error message, so I made this patch which makes it produce the following error message:

invalid child element 'extensions' in track

This gives a reference to the unexpected thing and allows to easily find it in the xml file, meaning that if later somebody runs the parser on unsupported gpx file they'll have easier time figuring out the problem and reporting it.

P.S. I'm very much a rust newbie, so any improvement suggestions will be appreciated.
P.P.S. I still want to parse and process my files and also improve the experience of parsing GPX files with third-party extensions, but that'll go into another issue/PR.

@zudov zudov force-pushed the child-element-error branch 2 times, most recently from c3dd5b1 to 021a00e Compare February 25, 2018 13:57
@zudov
Copy link
Contributor Author

zudov commented Feb 25, 2018

The build was failed due to code formatting. I've now run cargo fmt and amended the commit.

This makes the error much more useful. Including the actual name of
a tag that caused problem makes it possible to figure out what's wrong
with the gpx file without having to go through the library code
finding out what did it expect.
brendanashworth pushed a commit to brendanashworth/rust-gpx that referenced this pull request Feb 26, 2018
This makes the error much more useful. Including the actual name of
a tag that caused problem makes it possible to figure out what's wrong
with the gpx file without having to go through the library code
finding out what did it expect.

PR: georust#7
@brendanashworth
Copy link
Member

@zudov, this is great, thank you for putting this together. I've merged it in as 92dbb56.

I look forward to any thoughts/work you have on enabling extensions! I'm not sure the best way to approach it so I'd love to see where you go. I opened #8 as a tracking issue for it.

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.

None yet

2 participants