Rearchitecture step 3 of 6. Sequence: #54 → #55 → 3 → #48 → #18 → #41.
Problem
Assertion constructors can fail, but their signature has no way to say so, so they panic:
func AssertBodyMatch(expPattern string) Assertion {
re := regexp.MustCompile(expPattern) // panics on user input
// …
}
Three constructors compile user-supplied regexes this way — AssertHeaderMatch (assertions.go:81), AssertBodyMatch (assertions.go:125), AssertRedirectMatch. A typo in a CI pipeline produces a raw Go stack trace and exit code 2.
#17 is the symptom; this is the cause. #17 could be closed by wrapping the three call sites in parseAssertionFlags, but the design would still be "a constructor that can fail has no error return", and the next fallible assertion added will reintroduce it.
Suggested fix
func AssertBodyMatch(expPattern string) (Assertion, error) {
re, err := regexp.Compile(expPattern)
if err != nil {
return nil, fmt.Errorf("--assert-body: %w", err)
}
// …
}
parseAssertionFlags collects errors and reports them as invalid arguments instead of crashing:
$ http-assert --assert-body '[unclosed' https://example.com
Error: Invalid value for --assert-body flag: error parsing regexp: missing closing ]: `[unclosed`
[exit=103]
Second change, same file: give assertions an identity
While the signatures are being touched, Assertion should stop being an opaque func(*httpResponse) error. Today a failure can only ever be a string, which is why structured output (#45) can't be added cheaply — there is nothing to serialize.
Before:
type Assertion func(res *httpResponse) error
After:
type Assertion interface {
Kind() string // "status", "header", "body", "redirect"
Check(res *httpResponse) *Failure
}
type Failure struct {
Kind string
Target string // header name, or ""
Expected any
Actual any
}
The functional style is genuinely good and worth preserving where it can be — this is about the result carrying structure, not about abandoning composability.
Why this is step 3
It should follow #55 (extract run()), because error-returning constructors need somewhere to return to — with os.Exit still buried in die(), the errors have no path to the surface. It should precede #48 so the end-to-end tests assert against the fixed exit codes rather than being written twice.
Related
Problem
Assertion constructors can fail, but their signature has no way to say so, so they panic:
Three constructors compile user-supplied regexes this way —
AssertHeaderMatch(assertions.go:81),AssertBodyMatch(assertions.go:125),AssertRedirectMatch. A typo in a CI pipeline produces a raw Go stack trace and exit code 2.#17 is the symptom; this is the cause. #17 could be closed by wrapping the three call sites in
parseAssertionFlags, but the design would still be "a constructor that can fail has no error return", and the next fallible assertion added will reintroduce it.Suggested fix
parseAssertionFlagscollects errors and reports them as invalid arguments instead of crashing:Second change, same file: give assertions an identity
While the signatures are being touched,
Assertionshould stop being an opaquefunc(*httpResponse) error. Today a failure can only ever be a string, which is why structured output (#45) can't be added cheaply — there is nothing to serialize.Before:
After:
The functional style is genuinely good and worth preserving where it can be — this is about the result carrying structure, not about abandoning composability.
Why this is step 3
It should follow #55 (extract
run()), because error-returning constructors need somewhere to return to — withos.Exitstill buried indie(), the errors have no path to the surface. It should precede #48 so the end-to-end tests assert against the fixed exit codes rather than being written twice.Related