Copilot commented on code in PR #8811:
URL:
https://github.com/apache/incubator-devlake/pull/8811#discussion_r3010721166
##########
backend/plugins/gh-copilot/tasks/report_download_helper.go:
##########
@@ -69,7 +85,120 @@ type reportMetadataResponse struct {
ReportEndDay string `json:"report_end_day"`
}
+func readReportMetadataBody(res *http.Response) ([]byte, errors.Error) {
+ body, readErr := io.ReadAll(res.Body)
+ res.Body.Close()
+ if readErr != nil {
+ return nil, errors.Default.Wrap(readErr, "failed to read report
metadata")
+ }
+ return body, nil
+}
+
+func logReportMetadataParseError(body []byte, err error, logger log.Logger) {
+ if logger == nil {
+ return
+ }
+ snippet := string(body)
+ if len(snippet) > 200 {
+ snippet = snippet[:200]
+ }
+ logger.Error(err, "failed to parse report metadata, body=%s", snippet)
Review Comment:
`logReportMetadataParseError` logs a snippet of the raw metadata response
body. This metadata includes `download_links` (signed URLs), so on parse errors
this can leak sensitive, time-limited URLs into logs. Please redact/remove
`download_links` before logging, or log only non-sensitive fields (e.g.,
report_day/range and response size/status) instead of the raw body snippet.
```suggestion
logger.Error(err, "failed to parse report metadata, body_size=%d
bytes", len(body))
```
##########
backend/plugins/gh-copilot/tasks/report_download_helper.go:
##########
@@ -69,7 +85,120 @@ type reportMetadataResponse struct {
ReportEndDay string `json:"report_end_day"`
}
+func readReportMetadataBody(res *http.Response) ([]byte, errors.Error) {
+ body, readErr := io.ReadAll(res.Body)
+ res.Body.Close()
+ if readErr != nil {
+ return nil, errors.Default.Wrap(readErr, "failed to read report
metadata")
+ }
+ return body, nil
+}
+
+func logReportMetadataParseError(body []byte, err error, logger log.Logger) {
+ if logger == nil {
+ return
+ }
+ snippet := string(body)
+ if len(snippet) > 200 {
+ snippet = snippet[:200]
+ }
+ logger.Error(err, "failed to parse report metadata, body=%s", snippet)
+}
+
+func reportMetadataRange(meta reportMetadataResponse) string {
+ if meta.ReportDay != "" {
+ return meta.ReportDay
+ }
+ if meta.ReportStartDay != "" && meta.ReportEndDay != "" {
+ return fmt.Sprintf("%s..%s", meta.ReportStartDay,
meta.ReportEndDay)
+ }
+ return ""
+}
+
+func logMissingDownloadLinks(meta reportMetadataResponse, logger log.Logger) {
+ if logger == nil || len(meta.DownloadLinks) != 0 {
+ return
+ }
+ reportRange := reportMetadataRange(meta)
+ if reportRange != "" {
+ logger.Info("No download links for report day=%s, skipping",
reportRange)
Review Comment:
The log message says `report day=%s` but `reportMetadataRange()` can return
a range like `start..end`. Please adjust the wording (e.g.,
`report=%s`/`range=%s`) so logs remain accurate for both 1-day and multi-day
metadata.
```suggestion
logger.Info("No download links for report=%s, skipping",
reportRange)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
