Fix: Change account disabled code

[linux019]

If the account is disabled, the server returns a 503 error, which isn't correct.
The 503 indicates a service outage or infra issue, not an app-level error.

[postindustria-code]

In Go, break inside a switch leaves the switch, not the enclosing for. The old if / else if version broke out of the loop on the first matching error; this one runs to completion, so the last matching error wins instead of the first. The three break statements below are now dead code (a Go switch does not fall through).

I measured it with the same probe on the base commit a5002538f and on 680cb46fe:

errors passed in	base	this PR
[AccountDisabled, MalformedAcct]	503	500
[MalformedAcct, AccountDisabled]	500	403
[BlockedApp, MalformedAcct]	503	500

To be fair: this is latent, not a live regression — GetAccount returns immediately on each of these codes (account/account.go:21, :48, :53, :67) and BlockedApp returns on the spot (auction.go:1226), so a slice never carries two of them today. But the guard is gone and go vet does not catch it, so the next change that accumulates errors flips the behaviour silently.

Either drop the redundant break statements, or make the short-circuit explicit if it was intentional:

loop:
	for _, err := range errs {
		switch errortypes.ReadCode(err) {
		case errortypes.BlockedAppErrorCode, errortypes.AccountDisabledErrorCode:
			httpStatus = http.StatusForbidden
			metricsStatus = metrics.RequestStatusBlockedApp
			break loop
		case errortypes.MalformedAcctErrorCode:
			httpStatus = http.StatusInternalServerError
			metricsStatus = metrics.RequestStatusAccountConfigErr
			break loop
		}
	}

[postindustria-code]

Same as in auction.go:1923 — break inside a switch no longer exits the for, so the last matching error wins rather than the first, and the three break statements here are dead code. Latent today (the codes are mutually exclusive by construction), but the short-circuit the old code had is gone.

Worth pinning whichever semantics you settle on with a test: TestHandleError only ever passes single-error slices, so neither the old nor the new precedence rule is covered. A case with []error{&errortypes.AccountDisabled{}, &errortypes.AcctRequired{}} would do it.

[postindustria-code]

Minor consistency point: this site was left as an if / else if while auction.go and video_auction.go were converted to a switch in the same diff. That is not wrong — in fact this is the only one of the three that still short-circuits the loop correctly — but it does mean the three copies of this logic now differ in both style and behaviour.

Either convert all three or none; if you keep the switch elsewhere, this one is the reference for what the loop is supposed to do.

[postindustria-code]

Reviewed this as a change to the public /openrtb2/* contract rather than a refactor. The direction is right, and there is a stronger argument for it than the one in the description — see below. Three things I would like to see resolved before this merges; two of them are a few lines each.

Smoke checks on 680cb46fe: gofmt clean, go build ./... clean, go test ./endpoints/... passes.

The change itself is correct — and the description undersells it

PBS-Java already returns 403 for this condition: AuctionHandler.java maps BlocklistedAppException / BlocklistedAccountException to HttpResponseStatus.FORBIDDEN. So this is not a unilateral contract change — it removes a divergence between the two Prebid Server implementations. That is worth putting in the PR description; it is a much stronger justification than the semantics of 503 alone, and it is the first thing a host operator will want to know.

The operational argument is also real: 503 invites retry-with-backoff from clients and CDNs against an account that is disabled permanently.

1. break inside a switch no longer breaks the loop

endpoints/openrtb2/auction.go:1923 and endpoints/openrtb2/video_auction.go:431

The if / else if → switch conversion is presented as cosmetic, but in Go break inside a switch leaves the switch, not the enclosing for. The old code stopped at the first matching error; the new code runs the loop to completion, so the last matching error wins instead of the first. go vet does not flag this.

Measured with the same probe on the base commit a5002538f and on 680cb46fe:

errors passed in	base	this PR
[AccountDisabled, MalformedAcct]	503	500
[MalformedAcct, AccountDisabled]	500	403
[BlockedApp, MalformedAcct]	503	500
[AccountDisabled]	503	403

To be fair about the impact: this is latent today, not a live regression. The four codes are mutually exclusive by construction — GetAccount returns immediately on each of them (account/account.go:21, :48, :53, :67) and BlockedApp returns on the spot (auction.go:1226, video_auction.go:846), so a slice never carries two of them. But the guard that used to make that irrelevant is gone, the three break statements are now dead code that reads as if it short-circuits, and any future code that accumulates errors turns this into a silent behaviour change.

Also worth noting: amp_auction.go was left as an if, so of the three sites two lost the short-circuit and one kept it.

Suggested fix: either drop the now-dead break statements (a Go switch does not fall through, so they are redundant), or — if the short-circuit was intentional — make it explicit with a labelled break or by extracting the loop into a function that returns. A table-driven test with a multi-error slice would pin whichever semantics you choose; TestHandleError currently only passes single-error slices.

2. /event and /vtrack still return 503 for the same condition

endpoints/events/event.go:220

HandleAccountServiceErrors maps the same BlockedAppErrorCode || AccountDisabledErrorCode pair to http.StatusServiceUnavailable, and this PR does not touch it. It is called from event.go:104 and vtrack.go:103.

After this merges, the same disabled account gets 403 from /openrtb2/auction, /openrtb2/amp and /openrtb2/video, and 503 from /event and /vtrack — which is the exact inconsistency this PR sets out to remove, just in a different file. I checked the rest of endpoints/: that is the only remaining site, so it is a one-line change.

(setuid and cookie_sync return 400 on a blocked account, a third code again — but that predates this PR and I would leave it out of scope.)

3. The public docs currently promise 503, and there is no docs PR

prebid-server/endpoints/openrtb2/pbs-endpoint-auction.md in prebid.github.io says:

• HTTP 400 if the request is malformed, or
• HTTP 503 if the account or app specified in the request is blacklisted

This is documented public behaviour, so it needs a docs PR alongside this one. Since hosts may have alerting keyed on 503 here, it is also worth a line in the release notes.

Happy to re-review as soon as 1 and 2 are settled — and if leaving /event on 503 is deliberate, just say so and I will drop that point.

[linux019]

@postindustria-code Please don't expand the scope of the PR with AI-generated reports. Instead open a separate issue to address the issues