Skip to content

Conversation

@AndyCokeZero
Copy link

This PR addresses #33 by allowing oops.Code(...) to accept multiple code types (e.g. int and string) instead of only string.

Changes:

  • Change Code(code string) to Code(code any) on both oops.Code(...) and OopsErrorBuilder.Code(...).
  • Change OopsError.Code() return type from string to any.
  • Update structured logging (slog) and JSON/map serialization to carry non-string codes correctly.
  • Add/adjust tests to cover integer codes and nested error chains.
  • Update docs to reflect the new API signatures.

Note (Breaking Change):

  • OopsError.Code() now returns any (was string). Downstream callers should migrate using type assertions or a switch on the returned
    value.

Change Code(...) to accept any and Code() to return any.
Update docs/tests for int and string codes.
Fix stacktrace path normalization in module mode.

BREAKING CHANGE: OopsError.Code() now returns any (was string).
Copy link
Owner

@samber samber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contrib.

Just 1 comment

return e.code
},
)
func (o OopsError) Code() any {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use the recursive getDeepestErrorAttribute function ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

I initially avoided getDeepestErrorAttribute because it’s constrained to T comparable, and any isn’t comparable in general (e.g. map/slice). Also its coalescing logic assumes “zero value = empty”, which would incorrectly treat 0 as empty for int codes.

So I implemented a small custom recursion with an explicit emptiness rule: only nil and "" are considered “unset”, everything else (including 0) is a valid code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants