Skip to content

Commit

Permalink
fix rate limit error message, and rename err log field
Browse files Browse the repository at this point in the history
  • Loading branch information
@pkieltyka
pkieltyka committed Nov 28, 2023
1 parent a537bfd commit 062a68e
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 21 deletions.
8 changes: 4 additions & 4 deletions client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func (c *Client) FetchUserPermission(ctx context.Context, projectID uint64, user
userPerm, resourceAccess, err = c.permCache.GetUserPermission(ctx, projectID, userID)
if err != nil {
// log the error, but don't stop
c.logger.With("error", err).Error("FetchUserPermission failed to query the permCache")
c.logger.With("err", err).Error("FetchUserPermission failed to query the permCache")
}
}

Expand Down Expand Up @@ -186,7 +186,7 @@ func (c *Client) SpendQuota(ctx context.Context, quota *proto.AccessQuota, compu
}
if event != nil {
if _, err := c.quotaClient.NotifyEvent(ctx, quota.AccessKey.ProjectID, event); err != nil {
c.logger.With("error", err, "op", "use_access_key", "event", event).Error("-> quota control: failed to notify")
c.logger.With("err", err, "op", "use_access_key", "event", event).Error("-> quota control: failed to notify")
}
}
return true, nil
Expand Down Expand Up @@ -250,7 +250,7 @@ func (c *Client) Run(ctx context.Context) error {
// Start the sync
for range c.ticker.C {
if err := c.usage.SyncUsage(ctx, c.quotaClient, &c.service); err != nil {
c.logger.With("error", err, "op", "run").Error("-> quota control: failed to sync usage")
c.logger.With("err", err, "op", "run").Error("-> quota control: failed to sync usage")
continue
}
c.logger.With("op", "run").Info("-> quota control: synced usage")
Expand All @@ -269,7 +269,7 @@ func (c *Client) Stop(timeoutCtx context.Context) {
c.ticker.Stop()
}
if err := c.usage.SyncUsage(timeoutCtx, c.quotaClient, &c.service); err != nil {
c.logger.With("error", err, "op", "run").Error("-> quota control: failed to sync usage")
c.logger.With("err", err, "op", "run").Error("-> quota control: failed to sync usage")
}
c.logger.With("op", "stop").Info("-> quota control: stopped.")
}
Expand Down
13 changes: 10 additions & 3 deletions middleware/error.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func FailOnUnexpectedError(fn func(w http.ResponseWriter, err error)) func(w htt

func ContinueOnUnexpectedError(log logger.Logger, fn func(w http.ResponseWriter, err error)) func(w http.ResponseWriter, _ *http.Request, next http.Handler, err error) {
return func(w http.ResponseWriter, r *http.Request, next http.Handler, err error) {
log.With("error", err, "op", "quota").Error("-> quotacontrol: unexpected error")
log.With("err", err, "op", "quota").Error("-> quotacontrol: unexpected error")
if !shouldErrorContinue(log, err) {
fn(w, err)
return
Expand All @@ -28,7 +28,14 @@ func ContinueOnUnexpectedError(log logger.Logger, fn func(w http.ResponseWriter,

func ContinueOnAnyError(log logger.Logger, fn func(w http.ResponseWriter, err error)) func(w http.ResponseWriter, _ *http.Request, next http.Handler, err error) {
return func(w http.ResponseWriter, r *http.Request, next http.Handler, err error) {
log.With("error", err, "op", "quota").Error("-> quotacontrol: unexpected error -- continuing anyways")
log.With("err", err, "op", "quota").Error("-> quotacontrol: unexpected error -- continuing anyways")

// all errors are okay, except for ErrLimitExceeded
if errors.Is(err, proto.ErrLimitExceeded) {

This comment has been minimized.

Sign in to view

Copy link
@pkieltyka

pkieltyka Dec 4, 2023

Author Member

maybe this is being triggered for special keys..? I guess we should just check if special key, then always move forward.. but, i'm not sure if this is the area which is being hit.
fn(w, err)
return
}

next.ServeHTTP(w, r.WithContext(WithSkipRateLimit(r.Context())))
}
}
Expand All @@ -40,7 +47,7 @@ func shouldErrorContinue(log logger.Logger, err error) bool {
if !errors.As(err, &w) {
// Sample log of unexpected errors (every 10 seconds)
if time.Now().Second()%10 == 0 {
log.With("error", err).Error("quotacontrol: unexpected error, allowing all traffic")
log.With("err", err).Error("quotacontrol: unexpected error, allowing all traffic")
}
return true
}
Expand Down
28 changes: 19 additions & 9 deletions proto/legacy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package proto

import (
"encoding/json"
"errors"
"fmt"
"net/http"
)
Expand Down Expand Up @@ -43,15 +44,24 @@ var _StatusCode = map[int]string{
200: "",
}

func RespondWithLegacyError(w http.ResponseWriter, err error) {
rpcErr, ok := err.(WebRPCError)
if !ok {
rpcErr = ErrorWithCause(ErrWebrpcEndpoint, err)
}
func RespondWithLegacyError(rateLimitErrorMessage string) func(w http.ResponseWriter, err error) {
return func(w http.ResponseWriter, err error) {
rpcErr, ok := err.(WebRPCError)
if !ok {
rpcErr = ErrorWithCause(ErrWebrpcEndpoint, err)
}

w.Header().Set("Content-Type", "application/json")
w.WriteHeader(rpcErr.HTTPStatus)
if errors.Is(err, ErrLimitExceeded) {
rpcErr = ErrLimitExceeded
if rateLimitErrorMessage != "" {
rpcErr.Message = rateLimitErrorMessage
}
}

respBody, _ := json.Marshal(rpcErr.getLegacyPayload())
w.Write(respBody)
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(rpcErr.HTTPStatus)

respBody, _ := json.Marshal(rpcErr.getLegacyPayload())
w.Write(respBody)
}
}
10 changes: 5 additions & 5 deletions quotacontrol.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func (q qcHandler) GetAccessQuota(ctx context.Context, accessKey string, now tim
}

if err := q.quotaCache.SetAccessQuota(ctx, &record); err != nil {
q.log.With("error", err).Error("quotacontrol: failed to set access quota in cache")
q.log.With("err", err).Error("quotacontrol: failed to set access quota in cache")
}

return &record, nil
Expand Down Expand Up @@ -207,12 +207,12 @@ func (q qcHandler) SetAccessLimit(ctx context.Context, projectID uint64, config
}
accessKeys, err := q.ListAccessKeys(ctx, projectID, proto.Ptr(true), nil)
if err != nil {
q.log.With("error", err).Error("quotacontrol: failed to list access keys")
q.log.With("err", err).Error("quotacontrol: failed to list access keys")
return true, nil
}
for _, access := range accessKeys {
if err := q.quotaCache.DeleteAccessKey(ctx, access.AccessKey); err != nil {
q.log.With("error", err).Error("quotacontrol: failed to delete access quota from cache")
q.log.With("err", err).Error("quotacontrol: failed to delete access quota from cache")
}
}
return true, nil
Expand Down Expand Up @@ -407,7 +407,7 @@ func (q qcHandler) GetUserPermission(ctx context.Context, projectID uint64, user
}

if err := q.permCache.SetUserPermission(ctx, projectID, userID, perm, access); err != nil {
q.log.With("error", err).Error("quotacontrol: failed to set user perm in cache")
q.log.With("err", err).Error("quotacontrol: failed to set user perm in cache")
}

return perm, access, nil
Expand All @@ -420,7 +420,7 @@ func (q qcHandler) updateAccessKey(ctx context.Context, access *proto.AccessKey)
}

if err := q.quotaCache.DeleteAccessKey(ctx, access.AccessKey); err != nil {
q.log.With("error", err).Error("quotacontrol: failed to delete access quota from cache")
q.log.With("err", err).Error("quotacontrol: failed to delete access quota from cache")
}

return access, nil
Expand Down

0 comments on commit 062a68e

Please sign in to comment.