Skip to content

Commit

Permalink
Log internal errors rather than returning them to requester (#1353)
Browse files Browse the repository at this point in the history
* Log internal errors rather than returning them to requester

* redact URLs in errors
  • Loading branch information
mjh1 authored Aug 12, 2024
1 parent 0fab642 commit 3d6ad06
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 25 deletions.
4 changes: 3 additions & 1 deletion clients/broadcaster_local.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"
"io"
"net/url"

"github.com/livepeer/catalyst-api/log"
)

// Currently only implemented by LocalBroadcasterClient
Expand All @@ -20,7 +22,7 @@ type LocalBroadcasterClient struct {
func NewLocalBroadcasterClient(broadcasterURL string) (BroadcasterClient, error) {
u, err := url.Parse(broadcasterURL)
if err != nil {
return &LocalBroadcasterClient{}, fmt.Errorf("error parsing local broadcaster URL %q: %s", broadcasterURL, err)
return &LocalBroadcasterClient{}, fmt.Errorf("error parsing local broadcaster URL %q: %s", log.RedactURL(broadcasterURL), err)
}
return &LocalBroadcasterClient{
broadcasterURL: *u,
Expand Down
4 changes: 2 additions & 2 deletions clients/callback_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,12 @@ func (pcc *PeriodicCallbackClient) doWithRetries(r *http.Request) error {

resp, err := metrics.MonitorRequest(metrics.Metrics.TranscodingStatusUpdate, pcc.httpClient, r)
if err != nil {
return fmt.Errorf("failed to send callback to %q. Error: %s", r.URL.String(), err)
return fmt.Errorf("failed to send callback to %q. Error: %s", r.URL.Redacted(), err)
}
defer resp.Body.Close()

if resp.StatusCode >= 400 {
return fmt.Errorf("failed to send callback to %q. HTTP Code: %d", r.URL.String(), resp.StatusCode)
return fmt.Errorf("failed to send callback to %q. HTTP Code: %d", r.URL.Redacted(), resp.StatusCode)
}

return nil
Expand Down
21 changes: 6 additions & 15 deletions errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@ import (
"errors"
"fmt"
"net/http"
"strings"

"github.com/cenkalti/backoff/v4"
"github.com/livepeer/catalyst-api/log"
"github.com/xeipuuv/gojsonschema"
)

type APIError struct {
Expand All @@ -23,7 +21,12 @@ func writeHttpError(w http.ResponseWriter, msg string, status int, err error) AP

var errorDetail string
if err != nil {
errorDetail = err.Error()
switch status {
case http.StatusInternalServerError:
log.LogNoRequestID("returning HTTP 500", "http_error_msg", msg, "err", err)
default:
errorDetail = err.Error()
}
}

if err := json.NewEncoder(w).Encode(map[string]string{"error": msg, "error_detail": errorDetail}); err != nil {
Expand Down Expand Up @@ -53,18 +56,6 @@ func WriteHTTPInternalServerError(w http.ResponseWriter, msg string, err error)
return writeHttpError(w, msg, http.StatusInternalServerError, err)
}

func WriteHTTPBadBodySchema(where string, w http.ResponseWriter, errors []gojsonschema.ResultError) APIError {
sb := strings.Builder{}
sb.WriteString("Body validation error in ")
sb.WriteString(where)
sb.WriteString(" ")
for i := 0; i < len(errors); i++ {
sb.WriteString(errors[i].String())
sb.WriteString(" ")
}
return writeHttpError(w, sb.String(), http.StatusBadRequest, nil)
}

type unretriableError struct{ error }

// Unretriable returns an error that should be treated as final. This effectively means that the error stops backoff
Expand Down
2 changes: 1 addition & 1 deletion pipeline/coordinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ func (c *Coordinator) StartUploadJob(p UploadJobPayload) {
// Use new clipped manifest as the source URL
clipSourceURL, err := clients.ClipInputManifest(p.RequestID, sourceURL.String(), p.ClipTargetURL.String(), p.ClipStrategy.StartTime, p.ClipStrategy.EndTime)
if err != nil {
return fmt.Errorf("clipping failed: %s %w", sourceURL, err)
return fmt.Errorf("clipping failed: %s %w", sourceURL.Redacted(), err)
}
sourceURL = clipSourceURL
return nil
Expand Down
4 changes: 2 additions & 2 deletions pipeline/ffmpeg.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ func (f *ffmpeg) probeSourceSegment(requestID string, seg *m3u8.MediaSegment, so
}
probeURL, err := clients.SignURL(u)
if err != nil {
return fmt.Errorf("failed to create signed url for %s: %w", u, err)
return fmt.Errorf("failed to create signed url for %s: %w", u.Redacted(), err)
}
if err := backoff.Retry(func() error {
_, err = f.probe.ProbeFile(requestID, probeURL)
Expand Down Expand Up @@ -350,7 +350,7 @@ func copyFileToLocalTmpAndSegment(job *JobInfo) (string, error) {
defer cancel()
_, err = clients.CopyFile(timeout, job.SignedSourceURL, localSourceFile.Name(), "", job.RequestID)
if err != nil {
return fmt.Errorf("failed to copy file (%s) locally for segmenting: %s", job.SignedSourceURL, err)
return fmt.Errorf("failed to copy file (%s) locally for segmenting: %s", log.RedactURL(job.SignedSourceURL), err)
}
return nil
}, retries(6)); err != nil {
Expand Down
8 changes: 4 additions & 4 deletions transcode/transcode.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func RunTranscodeProcess(transcodeRequest TranscodeSegmentRequest, streamName st
lastSegment := sourceSegmentURLs[len(sourceSegmentURLs)-1]
lastSegmentURL, err := clients.SignURL(lastSegment.URL)
if err != nil {
return outputs, segmentsCount, fmt.Errorf("failed to create signed url for last segment %s: %w", lastSegment.URL, err)
return outputs, segmentsCount, fmt.Errorf("failed to create signed url for last segment %s: %w", lastSegment.URL.Redacted(), err)
}
// ignore the following probe errors when checking the last segment
var ignoreProbeErrs = []string{
Expand Down Expand Up @@ -397,13 +397,13 @@ func RunTranscodeProcess(transcodeRequest TranscodeSegmentRequest, streamName st
var err error
probeURL, err = clients.SignURL(mp4TargetUrl)
if err != nil {
return outputs, segmentsCount, fmt.Errorf("failed to create signed url for %s: %w", mp4TargetUrl, err)
return outputs, segmentsCount, fmt.Errorf("failed to create signed url for %s: %w", mp4TargetUrl.Redacted(), err)
}
}
// Populate OutputVideo structs with results from probing step to send back in final response to Studio
mp4Out, err = video.PopulateOutput(transcodeRequest.RequestID, video.Probe{}, probeURL, mp4Out)
if err != nil {
return outputs, segmentsCount, fmt.Errorf("failed to populate output for %s: %w", probeURL, err)
return outputs, segmentsCount, fmt.Errorf("failed to populate output for %s: %w", log.RedactURL(probeURL), err)
}
mp4Outputs = append(mp4Outputs, mp4Out)
}
Expand Down Expand Up @@ -657,7 +657,7 @@ func processTranscodeResult(

targetRenditionURL, err := url.JoinPath(targetOSURL.String(), profile.Name)
if err != nil {
return fmt.Errorf("error building rendition segment URL %q: %s", targetRenditionURL, err)
return fmt.Errorf("error building rendition segment URL %q: %s", log.RedactURL(targetRenditionURL), err)
}

if transcodeRequest.GenerateMP4 {
Expand Down

0 comments on commit 3d6ad06

Please sign in to comment.