Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
8a36b8a
fix(reddit): narrow pre-send classification; clarify manual-ad destin…
mrautela365 Jul 14, 2026
87447ae
fix(review): treat context errors during create as ambiguous, not pre…
mrautela365 Jul 14, 2026
d3c804d
fix(review): disable redirects so TLS errors prove pre-send (PR #27)
mrautela365 Jul 15, 2026
2fe56f5
fix(review): tighten redirect/TLS outcome classification (PR #27)
mrautela365 Jul 15, 2026
30715f4
fix(review): enforce no-follow unconditionally; align docs with code …
mrautela365 Jul 15, 2026
ce9971a
fix(review): drop TLS from pre-send classification, matching Meta (PR…
mrautela365 Jul 15, 2026
404f77d
docs(review): purge stale TLS-as-pre-send references (PR #27)
mrautela365 Jul 15, 2026
d67a1a3
docs(review): fix comment debris from TLS-classification cleanup (PR …
mrautela365 Jul 15, 2026
8a4d1fa
fix(review): operator keeps ALL other query params, not just non-utm_…
mrautela365 Jul 15, 2026
7e9a3cc
docs(review): align remaining ambiguity-contract comments with code (…
mrautela365 Jul 15, 2026
1e834a2
docs(review): operator drops trailing slash; narrow ctx-error doc (PR…
mrautela365 Jul 15, 2026
a5635d8
docs(review): transportError bullet + concept-doc "classified" wordin…
mrautela365 Jul 15, 2026
b038edb
test(review): pin the trailing-slash operator instruction (PR #27)
mrautela365 Jul 15, 2026
428ae70
docs(reddit): note shared no-follow policy covers fetchToken
mrautela365 Jul 15, 2026
e587090
fix(reddit): correct stale TLS/ambiguity comments in client.go
mrautela365 Jul 15, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 38 additions & 8 deletions internal/platform/reddit/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ package reddit
import (
"bytes"
"context"
"crypto/tls"
"encoding/base64"
"encoding/json"
"errors"
Expand Down Expand Up @@ -617,11 +618,20 @@ func createOutcomeAmbiguous(err error) bool {
}

// isPreSendDialError reports whether a httpClient.Do error clearly happened
// BEFORE any request bytes could have reached the server (DNS resolution failure,
// connection refused, or no route/network unreachable). Such a failure means the
// request was NOT sent, so it must NOT be treated as an ambiguous "may exist"
// transportError. A failure AFTER a connection is established (mid-flight timeout,
// unexpected EOF) is genuinely ambiguous and IS wrapped as transportError.
// BEFORE any request bytes could have reached the server, so the request was NOT
// sent and must NOT be treated as an ambiguous "may exist" transportError. It
// covers:
// - DNS resolution failure;
// - connection refused / no route / network unreachable (never connected);
// - TLS handshake / certificate errors (the secure channel was never
// established, so no request body was sent);
// - context cancellation/deadline that fired before or during connection setup
// (the caller aborted; treated as not-sent — the campaign path additionally
// checks ctx.Err() first, so a genuine caller-cancel is handled there).
//
// A failure AFTER a connection is established and bytes were sent (mid-flight
// timeout, unexpected EOF on the response) is genuinely ambiguous and IS wrapped
// as transportError.
func isPreSendDialError(err error) bool {
var dnsErr *net.DNSError
if errors.As(err, &dnsErr) {
Expand All @@ -630,6 +640,22 @@ func isPreSendDialError(err error) bool {
if errors.Is(err, syscall.ECONNREFUSED) || errors.Is(err, syscall.EHOSTUNREACH) || errors.Is(err, syscall.ENETUNREACH) {
return true
}
// TLS handshake / certificate verification failures: the secure channel was
// never established, so no request bytes were sent.
Comment thread
mrautela365 marked this conversation as resolved.
Outdated
var certErr *tls.CertificateVerificationError
if errors.As(err, &certErr) {
return true
}
var recordErr tls.RecordHeaderError
Comment thread
mrautela365 marked this conversation as resolved.
Outdated
if errors.As(err, &recordErr) {
return true
}
// Context cancellation/deadline surfacing from Do (e.g. cancelled between token
// refresh and the send, or during connection setup): the request did not
// complete a send that could have been applied.
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
return true
Comment thread
mrautela365 marked this conversation as resolved.
Outdated
Comment thread
mrautela365 marked this conversation as resolved.
Outdated
}
return false
}

Expand Down Expand Up @@ -1425,10 +1451,14 @@ func (c *Client) CreateCampaign(ctx context.Context, in CampaignInput) (*Campaig
} else {
variantCount := len(in.Variants)
if variantCount > 0 {
steps = append(steps, fmt.Sprintf("%d ad variant(s) ready -- create ads in Reddit Ads Manager with these headlines:", variantCount))
// The click URLs below show ONLY the generated utm_* parameters on the
// destination — any pre-existing query on the registration URL is omitted
// here to avoid persisting a secret in the returned steps. When building the
// ads manually, use YOUR registration URL (with its own query params intact)
// as the base and add these utm_* parameters, so the destination matches
// what an automated ad would use.
steps = append(steps, fmt.Sprintf("%d ad variant(s) ready -- create ads in Reddit Ads Manager with these headlines (append the shown utm_* params to your registration URL, keeping its existing query):", variantCount))
Comment thread
mrautela365 marked this conversation as resolved.
Outdated
Comment thread
mrautela365 marked this conversation as resolved.
Outdated
Comment thread
mrautela365 marked this conversation as resolved.
Outdated
for i := 0; i < variantCount; i++ {
// These are manual-action instructions returned to the caller; show the
// sanitized click URL (utm_* only) so no pre-existing secret leaks.
steps = append(steps, fmt.Sprintf(" Variant %d: %q -> %s", i+1, in.Variants[i].Headline, displayRedditUTMURL(in, i)))
}
} else {
Expand Down
33 changes: 33 additions & 0 deletions internal/platform/reddit/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ package reddit

import (
"context"
"crypto/tls"
"encoding/json"
"errors"
"fmt"
"io"
"net"
"net/http"
Expand All @@ -16,6 +18,7 @@ import (
"strings"
"sync"
"sync/atomic"
"syscall"
"testing"
"time"
)
Expand Down Expand Up @@ -908,6 +911,36 @@ func TestCreateCampaign_ConnRefusedNotUnconfirmed(t *testing.T) {
}
}

// TestIsPreSendDialError classifies which Do errors mean the request was NOT sent
// (so a create is NOT ambiguous): DNS, connection-refused/no-route, TLS handshake/
// certificate failures, and context cancellation/deadline. A generic post-connect
// failure (e.g. unexpected EOF) stays ambiguous (not pre-send).
func TestIsPreSendDialError(t *testing.T) {
preSend := []error{
&net.DNSError{Err: "no such host", Name: "x"},
syscall.ECONNREFUSED,
syscall.EHOSTUNREACH,
&tls.CertificateVerificationError{},
Comment thread
mrautela365 marked this conversation as resolved.
Outdated
context.Canceled,
context.DeadlineExceeded,
fmt.Errorf("wrapped: %w", context.Canceled),
Comment thread
mrautela365 marked this conversation as resolved.
Outdated
}
Comment thread
Copilot marked this conversation as resolved.
for _, e := range preSend {
if !isPreSendDialError(e) {
t.Errorf("isPreSendDialError(%v) = false, want true (pre-send / not sent)", e)
}
}
notPreSend := []error{
io.ErrUnexpectedEOF, // mid-flight after a connection was established
errors.New("some other error"),
}
for _, e := range notPreSend {
if isPreSendDialError(e) {
t.Errorf("isPreSendDialError(%v) = true, want false (ambiguous / post-connect)", e)
}
}
}

// TestCreateCampaign_2xxUndecodableIsUnconfirmed verifies a campaign create that
// returns 2xx with an undecodable body is treated as UNCONFIRMED (the mutation
// likely succeeded), returning a partial with the campaign name.
Expand Down
Loading