Repository navigation
feat(waferrors): add ToWafReturnCode - #239
Merged
Merged
Conversation
Add `waferrors.ToWafReturnCode`. It returns the ddwaf_run return code (`DDWAF_RET_CODE`) that an error comes from: -3 for `ErrInternal`, -2 for `ErrInvalidObject` and -1 for `ErrInvalidArgument`. It returns zero for every other error, including `nil`, an error that is not a `RunError`, and a `RunError` that go-libddwaf reports itself, such as `ErrTimeout`. `ToWafErrorCode` does not change. It returns the positive `RunError` value, not the libddwaf code. Callers that report the libddwaf code (for example the tracers' telemetry) had to copy the mapping.
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…7 for other errors `ToWafReturnCode` now returns the code to report for any error from go-libddwaf, not only for the three native errors: - `nil` gives 0. - `ErrInternal`, `ErrInvalidObject` and `ErrInvalidArgument` give -3, -2 and -1. - Any other error gives -127: `ErrOutOfMemory`, `ErrEmptyRuleAddresses`, an unknown `RunError`, an unknown return code, and errors that are not a `RunError`. - A timeout gives no code, and never hides another error. A tree that holds only timeouts gives 0. It walks the whole error tree (`Unwrap() error` and `Unwrap() []error`) with an iterative work list, visits the errors in the same order as `errors.As`, and returns the highest code (the closest to zero). An error that matches by its `As` method does not stop the search.
RomainMuller
added a commit
to DataDog/dd-trace-go
that referenced
this pull request
Nov 9, 2026
Use the head of DataDog/go-libddwaf#239 through a pseudo-version, in the root module only. This commit never lands.
… nodes An error whose As method returns true without setting the target must give -127, also when an earlier node of the tree matched a timeout. Declaring the target outside the walk loop would keep the timeout and give 0.
5 of 13 tasks
RomainMuller
added a commit
to DataDog/dd-trace-go
that referenced
this pull request
Nov 9, 2026
…waf#239 Use the head of DataDog/go-libddwaf#239 (95a3e52a8) through a pseudo-version, in the root module only, to test waferrors.ToWafReturnCode before go-libddwaf v5.1.0 is released. Do not merge this commit. Before the merge of this PR, merge main again, after the go-libddwaf upgrade of main, so that go.mod and go.sum come from main.
e-n-0
approved these changes
Nov 9, 2026
Zero means no error to report; it does not distinguish DDWAF_OK from DDWAF_MATCH. Remove a sentence that repeated the one before it.
1 of 4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds
waferrors.ToWafReturnCode(in error) int. It returns the code to report for an error from go-libddwaf. Recognized errors use the nativeddwaf_runreturn codes (DDWAF_RET_CODE).-127is the reporting fallback when no recognized code is available.ToWafErrorCodedoes not change.Contract
nilgives0.ErrInternal,ErrInvalidObjectandErrInvalidArgumentgive-3,-2and-1.-127:ErrOutOfMemory,ErrEmptyRuleAddresses, an unknownRunError, an unknown return code (ErrUnknownReturnCode), and any error that is not aRunError. libddwaf never returns-127; the Datadog tracers report it for errors from the bindings (RFC-1012). It is an unexported constant.ErrTimeout) gives no code, and it never hides another error of the tree. On a timeout,ddwaf_runreturnsDDWAF_OKorDDWAF_MATCH, or is not called at all (the time budget is used up in Go first). A tree that holds only timeouts gives0. Zero means no error to report; it does not distinguishDDWAF_OKfromDDWAF_MATCH. To know if a timeout occurred, useerrors.Is(err, ErrTimeout). It finds the timeout errors that go-libddwaf returns, but not a custom error that only has anAsmethod.Unwrap() errorandUnwrap() []error, for exampleerrors.Join), with an iterative work list (no recursion, no visited set). An error matches aRunErrorwith the same rules as one step oferrors.As(its type, or itsAs(any) boolmethod), but a match does not stop the search: the wrapped errors count too. The errors are visited in the same order aserrors.As(an error before the errors it wraps;Unwrap() []errorfrom left to right). When the tree has more than one code, the highest one wins (the closest to zero). Likeerrors.As, it does not detect a cycle.Results (the 39 cases of
TestToWafReturnCode)nil0ErrInternal/ErrInvalidObject/ErrInvalidArgument-3/-2/-1ErrTimeout; wrappedErrTimeout0ErrOutOfMemory;ErrEmptyRuleAddresses-127RunError(99);RunError(0);RunError(-3)-127ErrUnknownReturnCodewrapped with a code-127errors.New(...);ErrContextClosed;*PanicError-127ErrInvalidObjectwrapped once;ErrInvalidArgumentwrapped 3 times; wrapped 10,000 times-2;-1;-1Join(ErrInternal, ErrInvalidArgument, ErrInvalidObject)-1(highest)Join(error, ErrInternal)-3Join(error, ErrOutOfMemory)-127Join(ErrTimeout, ErrInternal);Join(ErrTimeout, ErrInvalidObject);Join(ErrInvalidArgument, ErrTimeout)-3;-2;-1(a timeout never hides a code)Join(ErrTimeout, ErrOutOfMemory);Join(ErrTimeout, RunError(99));Join(ErrTimeout, error)-127Join(ErrTimeout, wrapped ErrTimeout)0fmt.Errorf("%w: %w", ErrResultInvalidType, ErrInvalidObject)-2fmt.Errorf("run: %w", Join(error, wrapped ErrInvalidArgument))-1Asmethod givesErrInternal; givesErrTimeout-3;0AsgivesErrInternaland wrapsErrInvalidArgument-1(a match does not stop the search)AsgivesRunError(99)and wrapsErrInternal-3AsgivesErrTimeoutand wraps an error; wrapsErrInternal-127;-3Asreturnstruewithout setting the target-127Unwrap() []errorthat returns onlynil-127Join(ErrTimeout, <error whose As returns true without setting the target>)-127(the target is reset between nodes)TestToWafReturnCodeOrderchecks that the errors are visited in the same order aserrors.As.TestGoRunErrorReturnCodeRoundTrip(result_code_test.go, no build tag) checks the result against thebindingsconstants (WAFErrInternal,WAFErrInvalidObject,WAFErrInvalidArgument,WAFOK: same code;WAFMatch:0; an unknown code:-127).internal/bindingscannot be the source of the values: it importswaferrors.API compatibility
gorelease -base=v5.0.1:Motivation
dd-trace-go reports this code in its AppSec telemetry (
waf_errortag, RFC-1012) and had its own copy of the mapping. See DataDog/dd-trace-go#5827 and the review there ("Wouldn't be better to directly do this in go-libddwaf?"). With this function, dd-trace-go deletes its copy and its-127fallback, and it reports a real error even when the same run also timed out.