Skip to content

Commit 896b678

Browse files
Thomas StrombergThomas Stromberg
Thomas Stromberg
authored and
Thomas Stromberg
committed
more logging
1 parent 6c749ec commit 896b678

File tree

3 files changed

+150
-26
lines changed

3 files changed

+150
-26
lines changed

‎pkg/client/client.go‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,11 @@ const (
4242

4343
// Event represents a webhook event received from the server.
4444
type Event struct {
45-
Timestamp time.Time `json:"timestamp"`
46-
Raw map[string]any
47-
Type string `json:"type"`
48-
URL string `json:"url"`
45+
Timestamp time.Time `json:"timestamp"`
46+
Raw map[string]any
47+
Type string `json:"type"`
48+
URL string `json:"url"`
49+
DeliveryID string `json:"delivery_id,omitempty"`
4950
}
5051

5152
// Config holds the configuration for the client.
@@ -592,6 +593,10 @@ func (c *Client) readEvents(ctx context.Context, ws *websocket.Conn) error {
592593
}
593594
}
594595

596+
if deliveryID, ok := response["delivery_id"].(string); ok {
597+
event.DeliveryID = deliveryID
598+
}
599+
595600
c.mu.Lock()
596601
c.eventCount++
597602
eventNum := c.eventCount
@@ -604,18 +609,21 @@ func (c *Client) readEvents(ctx context.Context, ws *websocket.Conn) error {
604609
"timestamp", event.Timestamp.Format("15:04:05"),
605610
"type", event.Type,
606611
"url", event.URL,
612+
"delivery_id", event.DeliveryID,
607613
"raw", event.Raw)
608614
} else {
609615
if event.Type != "" && event.URL != "" {
610616
c.logger.Info("Event received",
611617
"timestamp", event.Timestamp.Format("15:04:05"),
612618
"event_number", eventNum,
613619
"type", event.Type,
614-
"url", event.URL)
620+
"url", event.URL,
621+
"delivery_id", event.DeliveryID)
615622
} else {
616623
c.logger.Info("Event received",
617624
"timestamp", event.Timestamp.Format("15:04:05"),
618625
"event_number", eventNum,
626+
"delivery_id", event.DeliveryID,
619627
"response", response)
620628
}
621629
}

‎pkg/srv/hub.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@ import (
1010
)
1111

1212
// Event represents a GitHub webhook event that will be broadcast to clients.
13-
// It contains the PR URL, timestamp, and event type from GitHub.
13+
// It contains the PR URL, timestamp, event type, and delivery ID from GitHub.
1414
type Event struct {
15-
URL string `json:"url"` // Pull request URL
16-
Timestamp time.Time `json:"timestamp"` // When the event occurred
17-
Type string `json:"type"` // GitHub event type (e.g., "pull_request")
15+
URL string `json:"url"` // Pull request URL
16+
Timestamp time.Time `json:"timestamp"` // When the event occurred
17+
Type string `json:"type"` // GitHub event type (e.g., "pull_request")
18+
DeliveryID string `json:"delivery_id,omitempty"` // GitHub webhook delivery ID (unique per webhook)
1819
}
1920

2021
// Hub manages WebSocket clients and event broadcasting.
@@ -106,17 +107,17 @@ func (h *Hub) Run(ctx context.Context) {
106107
select {
107108
case client.send <- msg.event:
108109
matched++
109-
log.Printf("delivered event to client: id=%s user=%s org=%s event_type=%s pr_url=%s",
110+
log.Printf("delivered event to client: id=%s user=%s org=%s event_type=%s pr_url=%s delivery_id=%s",
110111
client.ID, client.subscription.Username, client.subscription.Organization,
111-
msg.event.Type, msg.event.URL)
112+
msg.event.Type, msg.event.URL, msg.event.DeliveryID)
112113
default:
113114
dropped++
114115
log.Printf("dropped event for client %s: buffer full", client.ID)
115116
}
116117
}
117118
}
118-
log.Printf("broadcast event: type=%s matched=%d/%d clients, dropped=%d",
119-
msg.event.Type, matched, totalClients, dropped)
119+
log.Printf("broadcast event: type=%s delivery_id=%s matched=%d/%d clients, dropped=%d",
120+
msg.event.Type, msg.event.DeliveryID, matched, totalClients, dropped)
120121
}
121122
}
122123
}

‎pkg/webhook/handler.go‎

Lines changed: 128 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"crypto/sha256"
88
"encoding/hex"
99
"encoding/json"
10+
"fmt"
1011
"io"
1112
"log"
1213
"net/http"
@@ -135,33 +136,70 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
135136
return
136137
}
137138

138-
// Extract PR URL
139-
prURL := ExtractPRURL(eventType, payload)
140-
if prURL == "" {
141-
// Log full payload to understand the structure
139+
// For check events, always log the full payload to help with debugging
140+
if eventType == "check_run" || eventType == "check_suite" {
142141
payloadJSON, err := json.Marshal(payload)
143142
if err != nil {
144-
logger.Warn("failed to marshal payload for logging", logger.Fields{
143+
logger.Warn("failed to marshal check event payload", logger.Fields{
145144
"event_type": eventType,
146145
"delivery_id": deliveryID,
147146
"error": err.Error(),
148147
})
149148
} else {
150-
logger.Info("no PR URL found in event - full payload", logger.Fields{
149+
logger.Info("received check event - full payload for debugging", logger.Fields{
151150
"event_type": eventType,
152151
"delivery_id": deliveryID,
153152
"payload": string(payloadJSON),
154153
})
155154
}
155+
}
156+
157+
// Extract PR URL
158+
prURL := ExtractPRURL(eventType, payload)
159+
if prURL == "" {
160+
// For check events, try to extract commit SHA and look up associated PRs via API
161+
if eventType == "check_run" || eventType == "check_suite" {
162+
commitSHA := extractCommitSHA(eventType, payload)
163+
if commitSHA != "" {
164+
logger.Info("no PR URL in check event payload, will need API lookup", logger.Fields{
165+
"event_type": eventType,
166+
"delivery_id": deliveryID,
167+
"commit_sha": commitSHA,
168+
"note": "commit SHA can be used to query GitHub API: GET /repos/OWNER/REPO/commits/SHA/pulls",
169+
})
170+
} else {
171+
logger.Warn("check event has no PR URL and no commit SHA", logger.Fields{
172+
"event_type": eventType,
173+
"delivery_id": deliveryID,
174+
})
175+
}
176+
} else {
177+
// Log full payload to understand the structure (for non-check events)
178+
payloadJSON, err := json.Marshal(payload)
179+
if err != nil {
180+
logger.Warn("failed to marshal payload for logging", logger.Fields{
181+
"event_type": eventType,
182+
"delivery_id": deliveryID,
183+
"error": err.Error(),
184+
})
185+
} else {
186+
logger.Info("no PR URL found in event - full payload", logger.Fields{
187+
"event_type": eventType,
188+
"delivery_id": deliveryID,
189+
"payload": string(payloadJSON),
190+
})
191+
}
192+
}
156193
w.WriteHeader(http.StatusOK)
157194
return
158195
}
159196

160197
// Create and broadcast event
161198
event := srv.Event{
162-
URL: prURL,
163-
Timestamp: time.Now(),
164-
Type: eventType,
199+
URL: prURL,
200+
Timestamp: time.Now(),
201+
Type: eventType,
202+
DeliveryID: deliveryID,
165203
}
166204

167205
h.hub.Broadcast(event, payload)
@@ -220,53 +258,130 @@ func ExtractPRURL(eventType string, payload map[string]any) string {
220258
case "check_run", "check_suite":
221259
// Extract PR URLs from check events if available
222260
if checkRun, ok := payload["check_run"].(map[string]any); ok {
223-
if url := extractPRFromCheckEvent(checkRun, payload); url != "" {
261+
if url := extractPRFromCheckEvent(checkRun, payload, eventType); url != "" {
224262
return url
225263
}
226264
}
227265
if checkSuite, ok := payload["check_suite"].(map[string]any); ok {
228-
if url := extractPRFromCheckEvent(checkSuite, payload); url != "" {
266+
if url := extractPRFromCheckEvent(checkSuite, payload, eventType); url != "" {
229267
return url
230268
}
231269
}
270+
// Log when we can't extract PR URL from check event
271+
logger.Warn("no PR URL found in check event", logger.Fields{
272+
"event_type": eventType,
273+
"has_check_run": payload["check_run"] != nil,
274+
"has_check_suite": payload["check_suite"] != nil,
275+
"payload_keys": getPayloadKeys(payload),
276+
})
232277
default:
233278
// For other event types, no PR URL can be extracted
234279
}
235280
return ""
236281
}
237282

238283
// extractPRFromCheckEvent extracts PR URL from check_run or check_suite events.
239-
func extractPRFromCheckEvent(checkEvent map[string]any, payload map[string]any) string {
284+
func extractPRFromCheckEvent(checkEvent map[string]any, payload map[string]any, eventType string) string {
240285
prs, ok := checkEvent["pull_requests"].([]any)
241286
if !ok || len(prs) == 0 {
287+
logger.Info("check event has no pull_requests array", logger.Fields{
288+
"event_type": eventType,
289+
"has_pr_array": ok,
290+
"pr_array_length": len(prs),
291+
"check_event_keys": getMapKeys(checkEvent),
292+
})
242293
return ""
243294
}
244295

245296
pr, ok := prs[0].(map[string]any)
246297
if !ok {
298+
logger.Warn("pull_requests[0] is not a map", logger.Fields{
299+
"event_type": eventType,
300+
"pr_type": fmt.Sprintf("%T", prs[0]),
301+
})
247302
return ""
248303
}
249304

250305
// Try html_url first
251306
if htmlURL, ok := pr["html_url"].(string); ok {
307+
logger.Info("extracted PR URL from check event html_url", logger.Fields{
308+
"event_type": eventType,
309+
"pr_url": htmlURL,
310+
})
252311
return htmlURL
253312
}
254313

255314
// Fallback: construct from number
256315
num, ok := pr["number"].(float64)
257316
if !ok {
317+
logger.Warn("PR number not found in check event", logger.Fields{
318+
"event_type": eventType,
319+
"pr_keys": getMapKeys(pr),
320+
})
258321
return ""
259322
}
260323

261324
repo, ok := payload["repository"].(map[string]any)
262325
if !ok {
326+
logger.Warn("repository not found in payload", logger.Fields{
327+
"event_type": eventType,
328+
})
263329
return ""
264330
}
265331

266332
repoURL, ok := repo["html_url"].(string)
267333
if !ok {
334+
logger.Warn("repository html_url not found", logger.Fields{
335+
"event_type": eventType,
336+
"repo_keys": getMapKeys(repo),
337+
})
268338
return ""
269339
}
270340

271-
return repoURL + "/pull/" + strconv.Itoa(int(num))
341+
constructedURL := repoURL + "/pull/" + strconv.Itoa(int(num))
342+
logger.Info("constructed PR URL from check event", logger.Fields{
343+
"event_type": eventType,
344+
"pr_url": constructedURL,
345+
"pr_number": int(num),
346+
})
347+
return constructedURL
348+
}
349+
350+
// getPayloadKeys returns the keys from a payload map for logging.
351+
func getPayloadKeys(payload map[string]any) []string {
352+
keys := make([]string, 0, len(payload))
353+
for k := range payload {
354+
keys = append(keys, k)
355+
}
356+
return keys
357+
}
358+
359+
// getMapKeys returns the keys from a map for logging.
360+
func getMapKeys(m map[string]any) []string {
361+
keys := make([]string, 0, len(m))
362+
for k := range m {
363+
keys = append(keys, k)
364+
}
365+
return keys
366+
}
367+
368+
// extractCommitSHA extracts the commit SHA from check_run or check_suite events.
369+
func extractCommitSHA(eventType string, payload map[string]any) string {
370+
switch eventType {
371+
case "check_run":
372+
if checkRun, ok := payload["check_run"].(map[string]any); ok {
373+
if headSHA, ok := checkRun["head_sha"].(string); ok {
374+
return headSHA
375+
}
376+
}
377+
case "check_suite":
378+
if checkSuite, ok := payload["check_suite"].(map[string]any); ok {
379+
if headSHA, ok := checkSuite["head_sha"].(string); ok {
380+
return headSHA
381+
}
382+
}
383+
default:
384+
// Not a check event, no SHA to extract
385+
}
386+
return ""
272387
}

0 commit comments

Comments
 (0)