Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
912d2722f0 |
@@ -115,7 +115,8 @@ the record:
|
|||||||
- `ConsoleHandler` and `JSONHandler` return the error from their write to
|
- `ConsoleHandler` and `JSONHandler` return the error from their write to
|
||||||
stdout, wrapped, so `errors.Is` still matches the original
|
stdout, wrapped, so `errors.Is` still matches the original
|
||||||
- `WebhookHandler` returns an error when the request fails, and also when
|
- `WebhookHandler` returns an error when the request fails, and also when
|
||||||
the server answers with a status outside 2xx
|
the server answers with a status outside 2xx, a redirect included,
|
||||||
|
since it does not follow redirects
|
||||||
- `MultiplexHandler`, which simplelog installs as the default, passes the
|
- `MultiplexHandler`, which simplelog installs as the default, passes the
|
||||||
record to every handler it holds even after one of them fails, then
|
record to every handler it holds even after one of them fails, then
|
||||||
returns all their errors joined with `errors.Join` (nil if none failed)
|
returns all their errors joined with `errors.Join` (nil if none failed)
|
||||||
|
|||||||
@@ -24,7 +24,7 @@ files it depends on: .golangci.yml, REPO_POLICIES.md, .editorconfig,
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
* 2026-10-06: every handler now returns a failed delivery from Handle
|
* 2026-10-06: every handler now returns a failed delivery from `Handle`
|
||||||
instead of discarding it: console and JSON return the stdout write
|
instead of discarding it: console and JSON return the stdout write
|
||||||
error, the webhook also fails on a non-2xx answer, and the multiplex
|
error, the webhook also fails on a non-2xx answer, and the multiplex
|
||||||
delivers to every handler and returns their errors joined
|
delivers to every handler and returns their errors joined
|
||||||
|
|||||||
@@ -134,3 +134,29 @@ func TestWebhookHandlerReturnsErrorOnServerError(t *testing.T) {
|
|||||||
t.Fatalf("Handle returned %v, want an error for the 500 answer", err)
|
t.Fatalf("Handle returned %v, want an error for the 500 answer", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Following the redirect would resend the request as a GET without the
|
||||||
|
// record, and the GET is answered with 200, so only an error for the
|
||||||
|
// redirect itself tells the caller the record was lost.
|
||||||
|
func TestWebhookHandlerReturnsErrorOnRedirect(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(
|
||||||
|
func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
if r.URL.Path != "/moved" {
|
||||||
|
http.Redirect(w, r, "/moved", http.StatusFound)
|
||||||
|
}
|
||||||
|
},
|
||||||
|
))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
handler, err := NewWebhookHandler(server.URL)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("NewWebhookHandler: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
err = handler.Handle(context.Background(), errorTestRecord())
|
||||||
|
if !errors.Is(err, errWebhookStatus) {
|
||||||
|
t.Fatalf("Handle returned %v, want an error for the redirect", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
+15
-2
@@ -19,6 +19,7 @@ var errWebhookStatus = errors.New("webhook did not accept the record")
|
|||||||
// URL.
|
// URL.
|
||||||
type WebhookHandler struct {
|
type WebhookHandler struct {
|
||||||
webhookURL string
|
webhookURL string
|
||||||
|
client *http.Client
|
||||||
attrs handlerAttrs
|
attrs handlerAttrs
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -30,7 +31,17 @@ func NewWebhookHandler(webhookURL string) (*WebhookHandler, error) {
|
|||||||
return nil, fmt.Errorf("invalid webhook URL: %w", err)
|
return nil, fmt.Errorf("invalid webhook URL: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
return &WebhookHandler{webhookURL: webhookURL}, nil
|
return &WebhookHandler{
|
||||||
|
webhookURL: webhookURL,
|
||||||
|
client: &http.Client{
|
||||||
|
// Following a redirect can resend the request as a GET
|
||||||
|
// without the record, so Handle gets the redirect answer
|
||||||
|
// itself and returns it as an error.
|
||||||
|
CheckRedirect: func(*http.Request, []*http.Request) error {
|
||||||
|
return http.ErrUseLastResponse
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Enabled reports whether the handler processes records at the given
|
// Enabled reports whether the handler processes records at the given
|
||||||
@@ -48,6 +59,7 @@ func (w *WebhookHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
|
|||||||
|
|
||||||
return &WebhookHandler{
|
return &WebhookHandler{
|
||||||
webhookURL: w.webhookURL,
|
webhookURL: w.webhookURL,
|
||||||
|
client: w.client,
|
||||||
attrs: w.attrs.withAttrs(attrs),
|
attrs: w.attrs.withAttrs(attrs),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -61,6 +73,7 @@ func (w *WebhookHandler) WithGroup(name string) slog.Handler {
|
|||||||
|
|
||||||
return &WebhookHandler{
|
return &WebhookHandler{
|
||||||
webhookURL: w.webhookURL,
|
webhookURL: w.webhookURL,
|
||||||
|
client: w.client,
|
||||||
attrs: w.attrs.withGroup(name),
|
attrs: w.attrs.withGroup(name),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -86,7 +99,7 @@ func (w *WebhookHandler) Handle(ctx context.Context, record slog.Record) error {
|
|||||||
|
|
||||||
request.Header.Set("Content-Type", "application/json")
|
request.Header.Set("Content-Type", "application/json")
|
||||||
|
|
||||||
response, err := http.DefaultClient.Do(request)
|
response, err := w.client.Do(request)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user