Skip to content

Commit

Permalink
Propagate closing channel before resolve timeout
Browse files Browse the repository at this point in the history
  • Loading branch information
phillebaba committed Oct 18, 2023
1 parent aed20e2 commit 193c730
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- [#241](https://github.com/XenitAB/spegel/pull/241) Fix missing return on resolve error.
- [#223](https://github.com/XenitAB/spegel/pull/223) Propagate closing channel before resolve timeout.

### Security

Expand Down
19 changes: 12 additions & 7 deletions internal/registry/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,13 @@ func (r *Registry) handleMirror(c *gin.Context, key string) {
log := pkggin.FromContextOrDiscard(c)

// Resolve mirror with the requested key
resolveCtx, cancel := context.WithTimeout(c, r.resolveTimeout)
defer cancel()
resolveCtx = logr.NewContext(resolveCtx, log)
isExternal := r.isExternalRequest(c)
if isExternal {
log.Info("handling mirror request from external node", "path", c.Request.URL.Path, "ip", c.RemoteIP())
}
resolveCtx, cancel := context.WithTimeout(c, r.resolveTimeout)
defer cancel()
resolveCtx = logr.NewContext(resolveCtx, log)
mirrorCh, err := r.router.Resolve(resolveCtx, key, isExternal, r.resolveRetries)
if err != nil {
//nolint:errcheck // ignore
Expand All @@ -181,16 +181,16 @@ func (r *Registry) handleMirror(c *gin.Context, key string) {
}
for {
select {
case <-resolveCtx.Done():
// Resolving mirror has timed out meaning one could not be found.
case <-c.Done():
// Request has been closed by server or client. No use continuing.
//nolint:errcheck // ignore
c.AbortWithError(http.StatusNotFound, fmt.Errorf("could not resolve mirror for key: %s", key))
c.AbortWithError(http.StatusNotFound, fmt.Errorf("request closed for key: %s", key))
return
case mirror, ok := <-mirrorCh:
// Channel closed means no more mirrors will be received and max retries has been reached.
if !ok {
//nolint:errcheck // ignore
c.AbortWithError(http.StatusInternalServerError, fmt.Errorf("mirror resolution has been exhausted"))
c.AbortWithError(http.StatusNotFound, fmt.Errorf("mirror resolve retries exhausted for key: %s", key))
return
}

Expand Down Expand Up @@ -221,6 +221,11 @@ func (r *Registry) handleMirror(c *gin.Context, key string) {
}
log.V(5).Info("mirrored request", "path", c.Request.URL.Path, "url", u.String())
return
case <-resolveCtx.Done():
// Resolving mirror has timed out meaning one could not be found.
//nolint:errcheck // ignore
c.AbortWithError(http.StatusNotFound, fmt.Errorf("mirror resolve timed out for key: %s", key))
return
}
}
}
Expand Down
1 change: 1 addition & 0 deletions internal/routing/p2p.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ func (r *P2PRouter) Resolve(ctx context.Context, key string, allowSelf bool, cou
// Combine peer with registry port to create mirror endpoint.
peerCh <- fmt.Sprintf("http://%s:%s", v, r.registryPort)
}
close(peerCh)
}()
return peerCh, nil
}
Expand Down

0 comments on commit 193c730

Please sign in to comment.