Skip to content

ratelimits: Provide verbose user-facing rate limit errors#7653

Merged
beautifulentropy merged 7 commits into
mainfrom
ratelimits-verbose-errors
Aug 12, 2024
Merged

ratelimits: Provide verbose user-facing rate limit errors#7653
beautifulentropy merged 7 commits into
mainfrom
ratelimits-verbose-errors

Conversation

@beautifulentropy

@beautifulentropy beautifulentropy commented Aug 9, 2024 •
edited
Loading

Copy link
Copy Markdown
Member
  • Instruct callers to call *Decision.Result() to check the result of rate limit transactions
  • Preserve the Transaction within the resulting *Decision
  • Generate consistently formatted verbose errors using the metadata found in the *Decision
  • Fix broken key-value rate limits integration test in TestDuplicateFQDNRateLimit

Fixes #7577

@beautifulentropy beautifulentropy force-pushed the ratelimits-verbose-errors branch 2 times, most recently from df0c14e to 851b812 Compare August 9, 2024 16:29
@beautifulentropy beautifulentropy force-pushed the ratelimits-verbose-errors branch from 851b812 to fd2f5b1 Compare August 9, 2024 16:32
@beautifulentropy beautifulentropy marked this pull request as ready for review August 9, 2024 17:04
@beautifulentropy beautifulentropy requested a review from a team as a code owner August 9, 2024 17:04
@github-actions

github-actions Bot commented Aug 9, 2024

Copy link
Copy Markdown
Contributor

@beautifulentropy, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values.

aarongable
aarongable reviewed Aug 9, 2024
View reviewed changes
Comment thread ratelimits/limiter.go Outdated
Comment thread ratelimits/limiter.go Outdated
Comment thread ratelimits/limiter.go Outdated
Comment thread ratelimits/limiter.go
Comment thread ratelimits/limiter.go Outdated
Comment thread test/integration/ratelimit_test.go
beautifulentropy
beautifulentropy commented Aug 9, 2024
View reviewed changes
Comment thread ratelimits/limiter.go Outdated
@beautifulentropy beautifulentropy force-pushed the ratelimits-verbose-errors branch from 89a17ec to f819aab Compare August 12, 2024 18:15
aarongable
aarongable reviewed Aug 12, 2024
View reviewed changes
Comment thread ratelimits/gcra.go Outdated
@beautifulentropy beautifulentropy merged commit 6a3e9d7 into main Aug 12, 2024
@beautifulentropy beautifulentropy deleted the ratelimits-verbose-errors branch August 12, 2024 20:14
@beautifulentropy beautifulentropy mentioned this pull request Aug 15, 2024
Merged
beautifulentropy added a commit that referenced this pull request Aug 15, 2024
@beautifulentropy
ratelimits: Replace *Decision merging with always returning most rest…
31d0ff0 
…rictive (#7667)

Fix a bug added in #7653 which sometimes attributed an "Allowed"
`Transaction` to the amalgamated "Denied" `*Decision`. Instead, always
return the most restrictive `*Decision` in the batch.

Remove a debug `fmt.Printf()` call added in #7653
@sinback sinback mentioned this pull request Dec 2, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aarongable aarongable aarongable approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

2 participants

@beautifulentropy @aarongable