StationA / tilenol

Scalable, multi-backend geo vector tile server
MIT License
22 stars 6 forks source link

Remove empty geometries before marshaling. #54

Closed ccma14 closed 1 year ago

ccma14 commented 1 year ago

Description

This PR fixes an issue that causes tilenol to crash if a layer returned contains an empty geometry (stack trace below).

The issue is that tilenol attempts to Marshal layers into a gzipped MVT before storing it in the optional cache. However, Marshaling a Layer into a gzipped MVT via mvt.MarshalGzipped causes a panic if an empty geometry is encountered.

This one-line fix adds a call to Layer.removeEmpty after retrieving it from the source, but before marshaling it for storage in the cache. This function also removes empty geometries (not just too small ones), and hence the issue is avoided.

Testing Done

panic: geometry type not supported: <nil>

goroutine 258 [running]:
github.com/paulmach/orb/encoding/mvt.encodeGeometry(0x0, 0x0, 0x0, 0x2, 0x2, 0x0, 0x0, 0x0)
    /go/pkg/mod/github.com/paulmach/orb@v0.1.6/encoding/mvt/geometry.go:90 +0x1d37
github.com/paulmach/orb/encoding/mvt.Marshal(0xc0005c7cb8, 0x1, 0x1, 0x0, 0x7, 0xc0001e8130, 0xc0003880f0, 0xc000442e40)
    /go/pkg/mod/github.com/paulmach/orb@v0.1.6/encoding/mvt/marshal.go:60 +0x126
github.com/paulmach/orb/encoding/mvt.MarshalGzipped(0xc0005c7cb8, 0x1, 0x1, 0xc0001e8130, 0x7, 0x0, 0x0, 0xe)
    /go/pkg/mod/github.com/paulmach/orb@v0.1.6/encoding/mvt/marshal.go:21 +0x5a
github.com/stationa/tilenol.(*Server).getLayerData(0xc000183bf0, 0xdfb080, 0xc0002eeb00, 0xc0001e8130, 0x7, 0x0, 0x0, 0xe, 0x0, 0x1, ...)
    /go/src/github.com/stationa/tilenol/server.go:266 +0x345
github.com/stationa/tilenol.(*Server).getVectorTile.func2(0x0, 0x0)
    /go/src/github.com/stationa/tilenol/server.go:321 +0x308
golang.org/x/sync/errgroup.(*Group).Go.func1(0xc000388690, 0xc0003864d0)
    /go/pkg/mod/golang.org/x/sync@v0.0.0-20201020160332-67f06af15bc9/errgroup/errgroup.go:57 +0x59
created by golang.org/x/sync/errgroup.(*Group).Go
    /go/pkg/mod/golang.org/x/sync@v0.0.0-20201020160332-67f06af15bc9/errgroup/errgroup.go:54 +0x66
jerluc commented 1 year ago

Thanks @ccma14 this looks indeed like it should trim out empty geometries. I have a few questions:

ccma14 commented 1 year ago

To address the concerns in terms of run time overhead and side effects from calling removeEmpty, I've modified my patch such that we now use a small function that just filters out empty Geometries instead.

jerluc commented 1 year ago

Thanks @ccma14, I just need to quickly make format and get this merged down!