fix: create ratelimit namespace if it doesnt exist#3884
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughAdds explicit 410 Gone handling for deleted ratelimit namespaces, OpenAPI GoneErrorResponse schema, SWR-style namespace caching and create-on-miss logic in ratelimit handlers, auditlog and cache wiring, tests updates, new error code constant, and maps the code to HTTP 410 in middleware. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as v2/ratelimit.limit Handler
participant Cache as RatelimitNamespaceCache (SWR)
participant DB
participant Audit as AuditLogService
Client->>API: POST /v2/ratelimit.limit (namespace, identifier, limit, duration)
API->>Cache: Load(namespace) // SWR loader
alt Cache hit
Cache-->>API: namespace (includes overrides, DeletedAt)
else Cache miss / load
Cache->>DB: FindRatelimitNamespace (by id OR name)
DB-->>Cache: namespace row(s)
Cache-->>API: namespace
end
alt namespace.DeletedAt != nil
API-->>Client: 410 Gone (GoneErrorResponse, err:unkey:data:ratelimit_namespace_gone)
else namespace exists
API->>API: compute limit/duration (apply override if matched)
alt namespace missing -> create-on-miss
API->>API: RBAC check (root key perms)
API->>DB: Create namespace (RW tx)
DB-->>API: created namespace id
API->>Audit: Log RatelimitNamespaceCreateEvent
API->>Cache: Set(created namespace)
end
API->>DB: Rate-limit check/update
DB-->>API: Rate-limit result
API-->>Client: 200 OK (success, remaining, limit)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Flo4604
requested review from
MichaelUnkey,
chronark,
imeyer,
mcstepp,
ogzhanolguncu and
perkinsjr
as code owners
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
go/pkg/db/queries/ratelimit_namespace_find.sql (1)
12-17: Add supporting indexes for hot path.Given this sits on the ratelimit path, add/verify:
- ratelimit_namespaces: (workspace_id, id), (workspace_id, name)
- ratelimit_overrides: (namespace_id, deleted_at_m)
go/apps/api/routes/v2_ratelimit_limit/401_test.go (1)
26-42: Strengthen assertions for the error payload.Also assert the error envelope (meta + error) and an expected code to prevent regressions.
Example:
- res := testutil.CallRoute[handler.Request, handler.Response](h, route, headers, req) - require.Equal(t, http.StatusUnauthorized, res.Status) - require.NotNil(t, res.Body) + res := testutil.CallRoute[handler.Request, handler.Response](h, route, headers, req) + require.Equal(t, http.StatusUnauthorized, res.Status) + require.NotNil(t, res.Body) + require.Empty(t, res.Body.Data) // ensure no data on 401 + require.NotNil(t, res.Body.Error) + require.NotEmpty(t, res.Body.Meta.RequestId) // or equivalent meta field + // If available: require.Equal(t, "unkey.errors.unauthorized", res.Body.Error.Code)go/apps/api/openapi/spec/paths/v2/ratelimit/limit/index.yaml (1)
10-11: Update contract text: “Always returns 200” is now false with 410 Gone.Revise description to reflect non-200 cases to avoid breaking client expectations.
- **Important**: Always returns HTTP 200. Check the `success` field to determine if the request should proceed. + **Important**: Typically returns HTTP 200 and you must check the `success` field to decide if the request should proceed. + The endpoint can return non-200 in these cases: + - 401 Unauthorized: invalid/missing root key + - 403 Forbidden: root key lacks required permissions + - 410 Gone: namespace exists but is soft-deletedgo/apps/api/routes/v2_ratelimit_limit/400_test.go (1)
118-146: Duplicate subtest names; rename for clarityTwo subtests share the name "missing authorization header". Rename one to avoid ambiguity in test output.
- t.Run("missing authorization header", func(t *testing.T) { + t.Run("missing authorization header (bad request body shape)", func(t *testing.T) {Also applies to: 140-179
go/apps/api/routes/v2_ratelimit_limit/handler.go (1)
293-301: Consider logging overrideId in ClickHouse for observability.Adding the applied overrideId helps analyze behavior changes.
h.ClickHouse.BufferRatelimit(schema.RatelimitRequestV1{ RequestID: s.RequestID(), WorkspaceID: auth.AuthorizedWorkspaceID, Time: time.Now().UnixMilli(), NamespaceID: namespace.ID, Identifier: req.Identifier, Passed: result.Success, + OverrideID: func() string { if overrideId != "" { return overrideId }; return "" }(), })go/apps/api/routes/v2_ratelimit_delete_override/handler.go (1)
29-35: Add GoDoc for exported Handler and methods.Public types/functions must be documented per guidelines.
- type Handler struct { + // Handler implements /v2/ratelimit.deleteOverride. Loads the namespace via SWR cache, + // verifies RBAC, soft-deletes the override, audits the change, and invalidates cache. + type Handler struct {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (21)
apps/docs/errors/unkey/data/ratelimit_namespace_gone.mdx(1 hunks)go/apps/api/openapi/gen.go(1 hunks)go/apps/api/openapi/openapi-generated.yaml(2 hunks)go/apps/api/openapi/spec/error/GoneErrorResponse.yaml(1 hunks)go/apps/api/openapi/spec/paths/v2/ratelimit/limit/index.yaml(1 hunks)go/apps/api/routes/register.go(1 hunks)go/apps/api/routes/v2_ratelimit_delete_override/handler.go(3 hunks)go/apps/api/routes/v2_ratelimit_limit/200_test.go(2 hunks)go/apps/api/routes/v2_ratelimit_limit/400_test.go(2 hunks)go/apps/api/routes/v2_ratelimit_limit/401_test.go(1 hunks)go/apps/api/routes/v2_ratelimit_limit/403_test.go(2 hunks)go/apps/api/routes/v2_ratelimit_limit/410_test.go(3 hunks)go/apps/api/routes/v2_ratelimit_limit/accuracy_test.go(1 hunks)go/apps/api/routes/v2_ratelimit_limit/handler.go(7 hunks)go/pkg/codes/constants_gen.go(1 hunks)go/pkg/codes/unkey_data.go(2 hunks)go/pkg/db/acme_user_update_registered.sql_generated.go(0 hunks)go/pkg/db/queries/ratelimit_namespace_find.sql(1 hunks)go/pkg/db/ratelimit_namespace_find.sql_generated.go(1 hunks)go/pkg/zen/middleware_errors.go(1 hunks)go/pkg/zen/middleware_metrics.go(0 hunks)
💤 Files with no reviewable changes (2)
- go/pkg/zen/middleware_metrics.go
- go/pkg/db/acme_user_update_registered.sql_generated.go
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Follow comprehensive documentation guidelines for Go code as described in go/GO_DOCUMENTATION_GUIDELINES.md
Every public function/type in Go code must be documented
Prefer interfaces for testability in Go code
Use AIDEV-* comments for complex/important code in Go services
Files:
go/pkg/zen/middleware_errors.gogo/pkg/codes/unkey_data.gogo/pkg/codes/constants_gen.gogo/apps/api/routes/v2_ratelimit_limit/accuracy_test.gogo/pkg/db/ratelimit_namespace_find.sql_generated.gogo/apps/api/routes/v2_ratelimit_limit/401_test.gogo/apps/api/routes/v2_ratelimit_limit/403_test.gogo/apps/api/routes/v2_ratelimit_limit/200_test.gogo/apps/api/routes/v2_ratelimit_limit/handler.gogo/apps/api/routes/v2_ratelimit_limit/410_test.gogo/apps/api/routes/register.gogo/apps/api/openapi/gen.gogo/apps/api/routes/v2_ratelimit_limit/400_test.gogo/apps/api/routes/v2_ratelimit_delete_override/handler.go
**/*.{env,js,ts,go}
📄 CodeRabbit inference engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
go/pkg/zen/middleware_errors.gogo/pkg/codes/unkey_data.gogo/pkg/codes/constants_gen.gogo/apps/api/routes/v2_ratelimit_limit/accuracy_test.gogo/pkg/db/ratelimit_namespace_find.sql_generated.gogo/apps/api/routes/v2_ratelimit_limit/401_test.gogo/apps/api/routes/v2_ratelimit_limit/403_test.gogo/apps/api/routes/v2_ratelimit_limit/200_test.gogo/apps/api/routes/v2_ratelimit_limit/handler.gogo/apps/api/routes/v2_ratelimit_limit/410_test.gogo/apps/api/routes/register.gogo/apps/api/openapi/gen.gogo/apps/api/routes/v2_ratelimit_limit/400_test.gogo/apps/api/routes/v2_ratelimit_delete_override/handler.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Use table-driven tests in Go
Organize Go integration tests with real dependencies
Organize Go tests by HTTP status codes
Files:
go/apps/api/routes/v2_ratelimit_limit/accuracy_test.gogo/apps/api/routes/v2_ratelimit_limit/401_test.gogo/apps/api/routes/v2_ratelimit_limit/403_test.gogo/apps/api/routes/v2_ratelimit_limit/200_test.gogo/apps/api/routes/v2_ratelimit_limit/410_test.gogo/apps/api/routes/v2_ratelimit_limit/400_test.go
🧬 Code graph analysis (10)
go/pkg/zen/middleware_errors.go (3)
go/pkg/codes/constants_gen.go (1)
UnkeyDataErrorsRatelimitNamespaceGone(88-88)go/apps/api/openapi/gen.go (4)
BadRequestErrorResponse(59-65)Meta(273-276)BadRequestErrorDetails(41-56)ValidationError(1934-1948)go/pkg/fault/wrapped.go (1)
UserFacingMessage(161-195)
go/pkg/codes/unkey_data.go (1)
go/pkg/codes/codes.go (3)
Code(97-106)SystemUnkey(32-32)CategoryUnkeyData(78-78)
go/pkg/codes/constants_gen.go (1)
go/pkg/urn/urn.go (1)
URN(12-19)
go/apps/api/routes/v2_ratelimit_limit/403_test.go (3)
go/pkg/testutil/http.go (2)
CallRoute(259-293)NewHarness(52-177)go/apps/api/routes/v2_ratelimit_limit/handler.go (2)
Request(31-31)Handler(35-44)go/apps/api/openapi/gen.go (1)
ForbiddenErrorResponse(132-138)
go/apps/api/routes/v2_ratelimit_limit/200_test.go (3)
go/apps/api/routes/register.go (1)
Register(58-556)go/pkg/testutil/http.go (1)
CallRoute(259-293)go/pkg/auditlog/events.go (1)
RatelimitNamespaceCreateEvent(26-26)
go/apps/api/routes/v2_ratelimit_limit/handler.go (9)
go/pkg/cache/interface.go (5)
Op(51-51)WriteValue(57-57)Noop(55-55)Key(34-36)Null(42-42)go/pkg/cache/scoped_key.go (1)
ScopedKey(44-57)go/pkg/db/retry.go (1)
WithRetry(28-67)go/pkg/db/ratelimit_namespace_find.sql_generated.go (1)
FindRatelimitNamespaceRow(37-45)go/pkg/db/handle_err_no_rows.go (1)
IsNotFound(8-10)go/pkg/db/tx.go (1)
TxWithResult(113-147)go/pkg/uid/uid.go (1)
RatelimitNamespacePrefix(25-25)go/pkg/auditlog/events.go (1)
RatelimitNamespaceCreateEvent(26-26)go/pkg/ptr/deref.go (1)
SafeDeref(35-44)
go/apps/api/routes/v2_ratelimit_limit/410_test.go (3)
go/apps/api/routes/v2_ratelimit_limit/handler.go (1)
Request(31-31)go/pkg/testutil/http.go (1)
CallRoute(259-293)go/apps/api/openapi/gen.go (1)
GoneErrorResponse(146-152)
go/apps/api/openapi/gen.go (1)
go/demo_api/main.go (1)
Error(90-95)
go/apps/api/routes/v2_ratelimit_limit/400_test.go (1)
go/internal/services/caches/caches.go (1)
Caches(15-27)
go/apps/api/routes/v2_ratelimit_delete_override/handler.go (8)
go/pkg/db/tx.go (2)
TxWithResult(113-147)Tx(196-201)go/pkg/cache/scoped_key.go (1)
ScopedKey(44-57)go/pkg/cache/interface.go (2)
Key(34-36)Null(42-42)go/pkg/db/retry.go (1)
WithRetry(28-67)go/pkg/db/ratelimit_namespace_find.sql_generated.go (1)
FindRatelimitNamespaceRow(37-45)go/internal/services/caches/op.go (1)
DefaultFindFirstOp(9-22)go/pkg/db/handle_err_no_rows.go (1)
IsNotFound(8-10)go/internal/services/caches/caches.go (1)
New(67-109)
🪛 LanguageTool
apps/docs/errors/unkey/data/ratelimit_namespace_gone.mdx
[grammar] ~5-~5: Use correct spacing
Context: ...limit Namespace Gone --- ## Description This error occurs when you attempt to us...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~7-~7: There might be a mistake here.
Context: ... error occurs when you attempt to use a ratelimit namespace that has been deleted. Once a...
(QB_NEW_EN_OTHER)
[grammar] ~7-~7: Use correct spacing
Context: ...e restored through the API or dashboard. ## Error Code `unkey/data/ratelimit_namesp...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~9-~9: Use correct spacing
Context: ...ugh the API or dashboard. ## Error Code unkey/data/ratelimit_namespace_gone ## HTTP Status Code 410 Gone ## Cause ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~13-~13: Use correct spacing
Context: ...mit_namespace_gone ## HTTP Status Code 410 Gone` ## Cause The ratelimit namespace you're tr...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~17-~17: Use correct spacing
Context: ...# HTTP Status Code 410 Gone ## Cause The ratelimit namespace you're trying to...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~19-~19: There might be a mistake here.
Context: ...o longer available. This is a permanent deletion and the namespace cannot be recovered. ...
(QB_NEW_EN_OTHER)
[grammar] ~19-~19: Use correct spacing
Context: ...n and the namespace cannot be recovered. ## Resolution Contact [support@unke...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~21-~21: Use correct spacing
Context: ...pace cannot be recovered. ## Resolution Contact support@unkey.com with your workspace ID and namespace name if you need this namespace restored. ## Prevention To avoid accidentally deleti...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~27-~27: Use correct spacing
Context: ...mespace restored. ## Prevention To avoid accidentally deleting namespace...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~29-~29: There might be a mistake here.
Context: ... avoid accidentally deleting namespaces: - Use workspace permissions to restrict wh...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...ns to restrict who can delete namespaces - Review namespace deletion requests caref...
(QB_NEW_EN_OTHER)
[grammar] ~31-~31: There might be a mistake here.
Context: ...ion requests carefully before confirming ## Related - [Ratelimit Namespace Not Foun...
(QB_NEW_EN_OTHER)
[grammar] ~33-~33: Use correct spacing
Context: ... carefully before confirming ## Related - [Ratelimit Namespace Not Found](/errors/u...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[uncategorized] ~36-~36: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: .../data/ratelimit_namespace_not_found) - [Rate Limiting Documentation](/ratelimiting/introducti...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build / Build
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (18)
go/apps/api/openapi/spec/error/GoneErrorResponse.yaml (1)
1-16: Schema shape looks consistent with existing error envelopes.
required: [meta, error]with shared refs aligns with other error responses.go/pkg/db/queries/ratelimit_namespace_find.sql (1)
16-16: No-op cleanup.Trailing whitespace removal is fine.
go/apps/api/routes/v2_ratelimit_limit/accuracy_test.go (1)
66-67: Wiring Auditlogs into the handler is correct.Matches the new Handler contract and keeps the integration test realistic.
go/apps/api/routes/v2_ratelimit_limit/401_test.go (2)
21-22: Good: handler now receives Auditlogs.Keeps tests aligned with the updated Handler struct.
12-42: Ensure all Handler initializations include Auditlogs. I wasn’t able to automatically verify every&handler.Handler{}instantiation—please manually scan all&handler.Handler{}usages to confirm theAuditlogsfield is present.go/pkg/codes/constants_gen.go (1)
87-88: LGTM: new URN for ratelimit namespace GoneThe constant and comment are correct and consistent with existing naming.
go/pkg/codes/unkey_data.go (2)
51-53: LGTM: Gone code added to dataRatelimitNamespaceField naming and semantics align with constants and usage patterns.
124-125: LGTM: Data initialization includes GoneConsistent with Code{SystemUnkey, CategoryUnkeyData, "ratelimit_namespace_gone"}.
go/apps/api/routes/register.go (1)
118-126: LGTM: Audit logs wired into v2/ratelimit.limit handlerWiring looks consistent with other routes using Auditlogs.
go/apps/api/routes/v2_ratelimit_limit/400_test.go (2)
22-29: Wire Auditlogs into handler: LGTMCorrectly threads Auditlogs; no issues.
107-114: Ensure consistent handler wiring across testsAll essential deps (DB, Keys, Logger, Ratelimit, Cache, Auditlogs) are passed. Good.
go/apps/api/routes/v2_ratelimit_limit/403_test.go (1)
39-40: Auditlogs wiring: LGTMDependency injection consistent with handler signature.
go/apps/api/routes/v2_ratelimit_limit/200_test.go (2)
13-13: Import auditlog: LGTMNeeded for event assertions; fine.
32-33: Auditlogs injected into handler: LGTMMatches Register wiring.
go/apps/api/routes/v2_ratelimit_limit/410_test.go (1)
22-29: LGTM: route wiring now includes Auditlogs.Wiring Auditlogs into the handler matches the production path and ensures audit coverage in creation flows.
go/apps/api/routes/v2_ratelimit_limit/handler.go (1)
231-235: LGTM: returning Gone with proper error code.410 mapping via codes.Data.RatelimitNamespace.Gone aligns with the new OpenAPI response.
go/apps/api/routes/v2_ratelimit_delete_override/handler.go (2)
166-213: LGTM: transactional soft-delete with audit and fault wrapping.The write path looks correct and auditable; error mapping uses ServiceUnavailable on DB failure, which is appropriate.
119-125: Align soft-delete semantics for namespaces across endpoints or document divergenceThe
v2_ratelimit_limithandler returns 410 Gone for soft-deleted namespaces, but the override handlers (delete, get, list, set) all return 404 NotFound. Confirm the desired behavior:
- If soft-deleted namespaces should return Gone everywhere, update these handlers to use
codes.Data.RatelimitNamespace.Gone.URN()and adjust the public message.- fault.Code(codes.Data.RatelimitNamespace.NotFound.URN()), + fault.Code(codes.Data.RatelimitNamespace.Gone.URN()),- Otherwise (404 is intentional for management endpoints), add a note in the API docs explaining why management endpoints return 404 while the limit endpoint returns 410.
…yed/unkey into feat/autocreate_ratelimit_namespace
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
go/apps/api/openapi/openapi-generated.yaml (1)
2972-2976: Fix contract conflict in rate limit response schema
Update the description ingo/apps/api/openapi/spec/paths/v2/ratelimit/limit/V2RatelimitLimitResponseData.yaml(around line 39) as follows, then regenerate the client/server bindings and OpenAPI output:- You MUST check this field to determine if the request should proceed, as the endpoint always returns `HTTP 200` even when rate limited. + You MUST check this field to determine if the request should proceed. This endpoint returns HTTP 200 when the rate limit check is performed (whether successful or rate-limited). It may return 4xx/5xx when authentication, authorization, validation, or namespace state prevents performing the check.
♻️ Duplicate comments (9)
go/pkg/zen/middleware_errors.go (1)
92-92: Fix misleading comment headerThis block handles Gone errors, not “Request Entity Too Large.”
Apply:
- // Request Entity Too Large errors + // Gone errorsapps/docs/errors/unkey/data/ratelimit_namespace_gone.mdx (1)
7-7: Clarify soft-delete semantics and tighten wordingCall it “soft-deleted,” keep restoration path explicit, and punctuate bullets.
Apply:
-This error occurs when you attempt to use a ratelimit namespace that has been deleted. Once a namespace is deleted, it cannot be restored through the API or dashboard. +This error occurs when you attempt to use a ratelimit namespace that has been deleted (soft-deleted). It cannot be restored through the API or dashboard. -The ratelimit namespace you're trying to access was previously deleted and is no longer available through the API or dashboard. +The ratelimit namespace you're trying to access was previously soft-deleted and is not available through the API or dashboard. -Contact [support@unkey.com](mailto:support@unkey.com) with your workspace ID and namespace name if you need this namespace restored. +Contact [support@unkey.com](mailto:support@unkey.com) with your workspace ID and namespace name if you need this namespace restored or permanently deleted. -To avoid accidentally deleting namespaces: -- Restrict namespace deletion via workspace permissions -- Carefully review namespace-deletion requests before confirming +To avoid accidentally deleting namespaces: +- Restrict namespace deletion via workspace permissions. +- Carefully review namespace-deletion requests before confirming.Also applies to: 19-21, 29-36
go/apps/api/routes/v2_ratelimit_limit/403_test.go (2)
39-40: Strengthen 403 assertionsAlso assert title and request ID to lock the contract.
Apply:
res := testutil.CallRoute[handler.Request, openapi.ForbiddenErrorResponse](h, route, headers, req) // This should return a 403 Forbidden - user lacks create_namespace permission require.Equal(t, http.StatusForbidden, res.Status, "expected 403, got: %d, body: %s", res.Status, res.RawBody) require.NotNil(t, res.Body) -require.Contains(t, res.Body.Error.Detail, "create_namespace", "Error should mention missing create_namespace permission") +require.Contains(t, res.Body.Error.Detail, "create_namespace") +require.Equal(t, "Forbidden", res.Body.Error.Title) +require.NotEmpty(t, res.Body.Meta.RequestId)Also applies to: 61-67
69-116: Great negative check; add a couple more contract assertsKeep verifying not-created; also assert title and request ID.
Apply:
require.Equal(t, http.StatusForbidden, res.Status, "expected 403, got: %d, body: %s", res.Status, res.RawBody) require.NotNil(t, res.Body) -require.Contains(t, res.Body.Error.Detail, "create_namespace", "Error should mention missing create_namespace permission") +require.Contains(t, res.Body.Error.Detail, "create_namespace") +require.Equal(t, "Forbidden", res.Body.Error.Title) +require.NotEmpty(t, res.Body.Meta.RequestId)go/apps/api/openapi/spec/paths/v2/ratelimit/limit/index.yaml (1)
118-124: Add an example payload for 410 GoneHelps SDKs/tests validate the schema.
Apply:
"410": description: Gone - Namespace has been deleted content: application/json: schema: "$ref": "../../../../error/GoneErrorResponse.yaml" + examples: + softDeletedNamespace: + summary: Namespace soft-deleted + value: + meta: + requestId: req_01ZZZZZZZZZZZZZZZZZZZZZZZZ + error: + title: Resource Gone + type: https://docs.unkey.com/errors/unkey/data/ratelimit_namespace_gone + detail: This namespace has been deleted. Contact support to restore. + status: 410go/apps/api/routes/v2_ratelimit_limit/handler.go (4)
35-45: Add fuller GoDoc (and AIDEV overview) for exported HandlerDocument auto-create, 410-on-soft-delete, caching, and audit logging per guidelines.
Apply:
-// Handler implements zen.Route interface for the v2 ratelimit limit endpoint +// Handler handles POST /v2/ratelimit.limit. +// It loads the namespace via cache (SWR), auto-creates it on miss if the caller has +// ratelimit.*.create_namespace, returns 410 for soft-deleted namespaces, applies overrides, +// enforces rate limits, writes ClickHouse metrics, and emits an audit log on creation. +// AIDEV-OVERVIEW: SWR cache + conditional create + 410 Gone mapping + audit trail. type Handler struct {
244-251: Unify internal/public error phrasingAvoid leaking internals in public text.
Apply:
- fault.Internal("error matching overrides"), - fault.Public("Error matching ratelimit override"), + fault.Internal("failed to resolve ratelimit override"), + fault.Public("Unable to resolve rate limit configuration"),
117-123: Mitigate thundering herd on cache misses (optional)Wrap the load/create path with singleflight keyed by workspaceID:namespace to coalesce concurrent misses.
Additional code (outside this hunk):
// import "golang.org/x/sync/singleflight" var nsCreateGroup singleflight.GroupInside the create path:
_, err = nsCreateGroup.Do(auth.AuthorizedWorkspaceID+":"+req.Namespace, func() (any, error) { // existing TxWithResult create logic return nil, nil })
185-207: Audit actor fallback and cache under both keysProvide a sane ActorName when Name is NULL; cache by both namespace name and ID for later invalidations.
Apply:
- ActorName: auth.Key.Name.String, + ActorName: func() string { + if auth.Key.Name.Valid { + return auth.Key.Name.String + } + return "root key" + }(), @@ - h.RatelimitNamespaceCache.Set(ctx, cacheKey, result) + h.RatelimitNamespaceCache.Set(ctx, cacheKey, result) + h.RatelimitNamespaceCache.Set(ctx, cache.ScopedKey{ + WorkspaceID: auth.AuthorizedWorkspaceID, + Key: result.ID, + }, result)Also applies to: 212-214
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/docs/errors/unkey/data/ratelimit_namespace_gone.mdx(1 hunks)go/apps/api/openapi/openapi-generated.yaml(4 hunks)go/apps/api/openapi/spec/paths/v2/ratelimit/limit/index.yaml(3 hunks)go/apps/api/routes/v2_ratelimit_limit/403_test.go(2 hunks)go/apps/api/routes/v2_ratelimit_limit/handler.go(6 hunks)go/pkg/zen/middleware_errors.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Follow comprehensive documentation guidelines for Go code as described in go/GO_DOCUMENTATION_GUIDELINES.md
Every public function/type in Go code must be documented
Prefer interfaces for testability in Go code
Use AIDEV-* comments for complex/important code in Go services
Files:
go/pkg/zen/middleware_errors.gogo/apps/api/routes/v2_ratelimit_limit/handler.gogo/apps/api/routes/v2_ratelimit_limit/403_test.go
**/*.{env,js,ts,go}
📄 CodeRabbit inference engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
go/pkg/zen/middleware_errors.gogo/apps/api/routes/v2_ratelimit_limit/handler.gogo/apps/api/routes/v2_ratelimit_limit/403_test.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Use table-driven tests in Go
Organize Go integration tests with real dependencies
Organize Go tests by HTTP status codes
Files:
go/apps/api/routes/v2_ratelimit_limit/403_test.go
🧠 Learnings (3)
📓 Common learnings
Learnt from: Flo4604
PR: unkeyed/unkey#3884
File: go/apps/api/openapi/openapi-generated.yaml:1828-1845
Timestamp: 2025-08-29T13:35:44.890Z
Learning: In the Unkey system, soft-deleted resources (like ratelimit namespaces) cannot be restored through the API or dashboard, but they can be restored manually via direct database operations by the support team. The error documentation should reflect that users need to contact support for restoration, as the current GoneErrorResponse description does.
📚 Learning: 2025-08-29T13:35:44.890Z
Learnt from: Flo4604
PR: unkeyed/unkey#3884
File: go/apps/api/openapi/openapi-generated.yaml:1828-1845
Timestamp: 2025-08-29T13:35:44.890Z
Learning: In the Unkey system, soft-deleted resources (like ratelimit namespaces) cannot be restored through the API or dashboard, but they can be restored manually via direct database operations by the support team. The error documentation should reflect that users need to contact support for restoration, as the current GoneErrorResponse description does.
Applied to files:
apps/docs/errors/unkey/data/ratelimit_namespace_gone.mdxgo/apps/api/openapi/spec/paths/v2/ratelimit/limit/index.yamlgo/apps/api/openapi/openapi-generated.yaml
📚 Learning: 2025-08-29T13:48:43.293Z
Learnt from: Flo4604
PR: unkeyed/unkey#3884
File: go/apps/api/routes/v2_ratelimit_delete_override/handler.go:218-228
Timestamp: 2025-08-29T13:48:43.293Z
Learning: In the unkeyed/unkey codebase, when working with ratelimit namespace caching, req.Namespace parameter is either the namespace ID or the namespace name, so cache invalidation only needs to remove cache keys for namespace.ID and namespace.Name - no need to also remove req.Namespace as a separate key.
Applied to files:
apps/docs/errors/unkey/data/ratelimit_namespace_gone.mdxgo/apps/api/routes/v2_ratelimit_limit/handler.go
🧬 Code graph analysis (3)
go/pkg/zen/middleware_errors.go (3)
go/pkg/codes/constants_gen.go (1)
UnkeyDataErrorsRatelimitNamespaceGone(88-88)go/apps/api/openapi/gen.go (3)
GoneErrorResponse(146-152)Meta(273-276)BaseError(68-80)go/pkg/fault/wrapped.go (1)
UserFacingMessage(161-195)
go/apps/api/routes/v2_ratelimit_limit/handler.go (12)
go/pkg/cache/scoped_key.go (1)
ScopedKey(44-57)go/pkg/db/retry.go (1)
WithRetry(28-67)go/pkg/db/ratelimit_namespace_find.sql_generated.go (1)
FindRatelimitNamespaceRow(37-45)go/internal/services/caches/op.go (1)
DefaultFindFirstOp(9-22)go/pkg/db/handle_err_no_rows.go (1)
IsNotFound(8-10)go/internal/services/keys/options.go (1)
WithPermissions(47-52)go/pkg/rbac/query.go (1)
T(84-90)go/pkg/rbac/permissions.go (2)
Tuple(175-184)CreateNamespace(77-77)go/pkg/db/tx.go (1)
TxWithResult(113-147)go/pkg/uid/uid.go (2)
New(96-126)RatelimitNamespacePrefix(25-25)go/pkg/auditlog/events.go (1)
RatelimitNamespaceCreateEvent(26-26)go/pkg/ptr/deref.go (1)
SafeDeref(35-44)
go/apps/api/routes/v2_ratelimit_limit/403_test.go (3)
go/pkg/testutil/http.go (2)
CallRoute(259-293)NewHarness(52-177)go/apps/api/routes/v2_ratelimit_limit/handler.go (2)
Request(32-32)Handler(36-45)go/pkg/db/ratelimit_namespace_find.sql_generated.go (1)
FindRatelimitNamespaceParams(32-35)
🪛 LanguageTool
apps/docs/errors/unkey/data/ratelimit_namespace_gone.mdx
[grammar] ~5-~5: Use correct spacing
Context: ...limit Namespace Gone --- ## Description This error occurs when you attempt to us...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~7-~7: There might be a mistake here.
Context: ... error occurs when you attempt to use a ratelimit namespace that has been deleted. Once a...
(QB_NEW_EN_OTHER)
[grammar] ~7-~7: Use correct spacing
Context: ...e restored through the API or dashboard. ## Error Code `unkey/data/ratelimit_namesp...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~9-~9: Use correct spacing
Context: ...ugh the API or dashboard. ## Error Code unkey/data/ratelimit_namespace_gone ## HTTP Status Code 410 Gone ## Cause ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~13-~13: Use correct spacing
Context: ...mit_namespace_gone ## HTTP Status Code 410 Gone` ## Cause The ratelimit namespace you're tr...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~17-~17: Use correct spacing
Context: ...# HTTP Status Code 410 Gone ## Cause The ratelimit namespace you're trying to...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~19-~19: Use correct spacing
Context: ... available through the API or dashboard. ## Resolution Contact [support@unke...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~21-~21: Use correct spacing
Context: ...ugh the API or dashboard. ## Resolution Contact support@unkey.com with your workspace ID and namespace name if you need this namespace restored. ## Prevention To avoid accidentally deleti...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~27-~27: Use correct spacing
Context: ...mespace restored. ## Prevention To avoid accidentally deleting namespace...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~29-~29: There might be a mistake here.
Context: ... avoid accidentally deleting namespaces: - Restrict namespace deletion via workspac...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...space deletion via workspace permissions - Carefully review namespace-deletion requ...
(QB_NEW_EN_OTHER)
[grammar] ~31-~31: There might be a mistake here.
Context: ...pace-deletion requests before confirming ## Related - [Ratelimit Namespace Not Foun...
(QB_NEW_EN_OTHER)
[grammar] ~33-~33: Use correct spacing
Context: ...n requests before confirming ## Related - [Ratelimit Namespace Not Found](/errors/u...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[uncategorized] ~36-~36: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: .../data/ratelimit_namespace_not_found) - [Rate Limiting Documentation](/ratelimiting/introducti...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
go/pkg/zen/middleware_errors.go (1)
94-104: Correct 410 mapping to GoneErrorResponse — LGTMSwitching to openapi.GoneErrorResponse with BaseError is correct and aligns with the new schema.
go/apps/api/routes/v2_ratelimit_limit/handler.go (2)
259-260: Good: safe default costptr.SafeDeref with fallback 1 avoids nil deref and preserves semantics.
315-326: Helper separation — LGTMgetLimitAndDuration cleanly encapsulates override selection and defaults.
go/apps/api/openapi/openapi-generated.yaml (2)
5690-5690: Good: path now documents 200 vs 4xx/5xx for ratelimit.limit.This resolves prior ambiguity around non-200 outcomes (e.g., 410). No further changes here.
5770-5771: 200 response description is fine; keep it narrowly scoped to the success case.No action; the “check success” guidance remains useful.
|
We don’t have .com because we have to switch the domains etc and update every link. They don’t support 2 domains for support. |
|
ill just update the email to .dev |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/docs/errors/user/bad_request/request_body_too_large.mdx (1)
80-86: Tighten bullet punctuation for consistency.Add terminal periods to bullets.
-[Contact our support team](mailto:support@unkey.dev) and include: - - What you're building - - Why you need to send large requests - - An example of the data you're trying to send +[Contact our support team](mailto:support@unkey.dev) and include: + - What you're building. + - Why you need to send large requests. + - An example of the data you're trying to send.
♻️ Duplicate comments (10)
go/apps/api/routes/v2_ratelimit_limit/handler.go (5)
99-112: Harden Overrides decoding and avoid storing wildcards in DirectOverrides.Handle both []byte and string from the driver; keep wildcards out of the direct map.
- overrides := make([]db.FindRatelimitNamespaceLimitOverride, 0) - if overrideBytes, ok := response.Overrides.([]byte); ok && overrideBytes != nil { - err = json.Unmarshal(overrideBytes, &overrides) - if err != nil { - return result, err - } - } + overrides := make([]db.FindRatelimitNamespaceLimitOverride, 0) + switch v := response.Overrides.(type) { + case []byte: + if v != nil && len(v) > 0 { + if err := json.Unmarshal(v, &overrides); err != nil { + return result, err + } + } + case string: + if v != "" { + if err := json.Unmarshal([]byte(v), &overrides); err != nil { + return result, err + } + } + } - for _, override := range overrides { - result.DirectOverrides[override.Identifier] = override - if strings.Contains(override.Identifier, "*") { - result.WildcardOverrides = append(result.WildcardOverrides, override) - } - } + for _, o := range overrides { + if strings.Contains(o.Identifier, "*") { + result.WildcardOverrides = append(result.WildcardOverrides, o) + continue + } + result.DirectOverrides[o.Identifier] = o + }
191-207: Provide ActorName fallback for audit logs.Avoid empty actor names when Key.Name is NULL.
- ActorName: auth.Key.Name.String, + ActorName: func() string { if auth.Key.Name.Valid { return auth.Key.Name.String }; return "root key" }(),
76-84: Coalesce concurrent misses with singleflight.Without it, many parallel misses stampede the DB then race to create.
I can wire golang.org/x/sync/singleflight around the loader keyed by workspaceID+namespace.
247-251: Unify public error text (non‑leaky, user‑friendly).Keep internal detail; make public message generic.
- fault.Internal("error matching overrides"), - fault.Public("Error matching ratelimit override"), + fault.Internal("failed to resolve ratelimit override"), + fault.Public("Unable to resolve rate limit configuration."),
35-45: Expand GoDoc to reflect new behaviors (auto-create, 410, audit, cache).Document the important semantics per guidelines; add a brief AIDEV note.
-// Handler implements zen.Route interface for the v2 ratelimit limit endpoint +// Handler implements the /v2/ratelimit.limit endpoint. +// - Loads the namespace via SWR cache; auto-creates on miss if the caller has create_namespace. +// - Returns 410 Gone for soft-deleted namespaces. +// - Applies overrides, buffers ClickHouse metrics, and writes audit logs on creation. +// AIDEV-NOTE: See creation flow below for duplicate-key handling and cache updates.go/apps/api/routes/v2_ratelimit_delete_override/handler.go (2)
83-96: Harden Overrides decoding and avoid storing wildcards in DirectOverrides.Mirror the limit handler fix to handle []byte/string and separate wildcard entries.
- overrides := make([]db.FindRatelimitNamespaceLimitOverride, 0) - if overrideBytes, ok := response.Overrides.([]byte); ok && overrideBytes != nil { - err = json.Unmarshal(overrideBytes, &overrides) - if err != nil { - return result, err - } - } + overrides := make([]db.FindRatelimitNamespaceLimitOverride, 0) + switch v := response.Overrides.(type) { + case []byte: + if v != nil && len(v) > 0 { + if err := json.Unmarshal(v, &overrides); err != nil { + return result, err + } + } + case string: + if v != "" { + if err := json.Unmarshal([]byte(v), &overrides); err != nil { + return result, err + } + } + }- for _, override := range overrides { - result.DirectOverrides[override.Identifier] = override - if strings.Contains(override.Identifier, "*") { - result.WildcardOverrides = append(result.WildcardOverrides, override) - } - } + for _, o := range overrides { + if strings.Contains(o.Identifier, "*") { + result.WildcardOverrides = append(result.WildcardOverrides, o) + continue + } + result.DirectOverrides[o.Identifier] = o + }
57-101: DRY: extract the SWR namespace loader shared with limit handler.Avoid duplicating decode/build logic; factor a small helper.
apps/docs/errors/unkey/data/ratelimit_namespace_gone.mdx (2)
35-36: Minor wording: hyphenate “Rate‑limiting”.Style/consistency only.
-- [Rate Limiting Documentation](/ratelimiting/introduction) +- [Rate‑limiting documentation](/ratelimiting/introduction)
7-8: Clarify “soft‑deleted” to avoid mixed signals with restoration via support.Make the status explicit and consistent with API behavior.
-This error occurs when you attempt to use a ratelimit namespace that has been deleted. Once a namespace is deleted, it cannot be restored through the API or dashboard. +This error occurs when you attempt to use a ratelimit namespace that has been deleted (soft‑deleted). It cannot be restored by you through the API or dashboard. @@ -The ratelimit namespace you're trying to access was previously deleted and is no longer available through the API or dashboard. +The ratelimit namespace you're trying to access was previously soft‑deleted and is not available via the API or dashboard.Also applies to: 19-21
go/apps/api/openapi/spec/paths/v2/ratelimit/limit/index.yaml (1)
118-123: Add a concrete 410 example payload (and mention contacting support to restore)This helps SDKs/tests and aligns with guidance that soft‑deleted namespaces require support to restore. The schema remains unchanged.
"410": description: Gone - Namespace has been deleted content: application/json: schema: "$ref": "../../../../error/GoneErrorResponse.yaml" + examples: + softDeletedNamespace: + summary: Namespace soft-deleted + value: + meta: + requestId: req_01ZZZZZZZZZZZZZZZZZZZZZZZZ + error: + code: unkey.data.errors.ratelimit_namespace_gone + message: "Namespace has been deleted. Contact support to restore."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/docs/errors/unkey/data/ratelimit_namespace_gone.mdx(1 hunks)apps/docs/errors/user/bad_request/request_body_too_large.mdx(1 hunks)go/apps/api/openapi/spec/paths/v2/ratelimit/limit/index.yaml(3 hunks)go/apps/api/routes/v2_ratelimit_delete_override/handler.go(3 hunks)go/apps/api/routes/v2_ratelimit_limit/handler.go(6 hunks)go/demo_api/main.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Follow comprehensive documentation guidelines for Go code as described in go/GO_DOCUMENTATION_GUIDELINES.md
Every public function/type in Go code must be documented
Prefer interfaces for testability in Go code
Use AIDEV-* comments for complex/important code in Go services
Files:
go/demo_api/main.gogo/apps/api/routes/v2_ratelimit_limit/handler.gogo/apps/api/routes/v2_ratelimit_delete_override/handler.go
**/*.{env,js,ts,go}
📄 CodeRabbit inference engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
go/demo_api/main.gogo/apps/api/routes/v2_ratelimit_limit/handler.gogo/apps/api/routes/v2_ratelimit_delete_override/handler.go
🧠 Learnings (5)
📚 Learning: 2025-08-22T12:46:58.510Z
Learnt from: perkinsjr
PR: unkeyed/unkey#3775
File: apps/dashboard/app/(app)/[workspaceId]/apis/[apiId]/keys/[keyAuthId]/_components/components/controls/index.tsx:1-1
Timestamp: 2025-08-22T12:46:58.510Z
Learning: The team at unkeyed/unkey is moving towards calling their v2 API directly and prefers not to refactor temporary code that will be replaced in the near future.
Applied to files:
go/demo_api/main.goapps/docs/errors/unkey/data/ratelimit_namespace_gone.mdx
📚 Learning: 2025-08-29T13:35:44.890Z
Learnt from: Flo4604
PR: unkeyed/unkey#3884
File: go/apps/api/openapi/openapi-generated.yaml:1828-1845
Timestamp: 2025-08-29T13:35:44.890Z
Learning: In the Unkey system, soft-deleted resources (like ratelimit namespaces) cannot be restored through the API or dashboard, but they can be restored manually via direct database operations by the support team. The error documentation should reflect that users need to contact support for restoration, as the current GoneErrorResponse description does.
Applied to files:
apps/docs/errors/unkey/data/ratelimit_namespace_gone.mdxgo/apps/api/routes/v2_ratelimit_limit/handler.gogo/apps/api/openapi/spec/paths/v2/ratelimit/limit/index.yaml
📚 Learning: 2025-08-29T13:48:43.293Z
Learnt from: Flo4604
PR: unkeyed/unkey#3884
File: go/apps/api/routes/v2_ratelimit_delete_override/handler.go:218-228
Timestamp: 2025-08-29T13:48:43.293Z
Learning: In the unkeyed/unkey codebase, when working with ratelimit namespace caching, req.Namespace parameter is either the namespace ID or the namespace name, so cache invalidation only needs to remove cache keys for namespace.ID and namespace.Name - no need to also remove req.Namespace as a separate key.
Applied to files:
apps/docs/errors/unkey/data/ratelimit_namespace_gone.mdxgo/apps/api/routes/v2_ratelimit_limit/handler.gogo/apps/api/routes/v2_ratelimit_delete_override/handler.go
📚 Learning: 2025-08-29T13:48:43.293Z
Learnt from: Flo4604
PR: unkeyed/unkey#3884
File: go/apps/api/routes/v2_ratelimit_delete_override/handler.go:218-228
Timestamp: 2025-08-29T13:48:43.293Z
Learning: The unkeyed/unkey codebase has identified a need for cache key normalization (forcing lowercase) to ensure consistent cache hit rates, with a preference for implementing a centralized cache.Key method to handle this standardization.
Applied to files:
go/apps/api/routes/v2_ratelimit_delete_override/handler.go
📚 Learning: 2025-08-14T16:25:48.167Z
Learnt from: Flo4604
PR: unkeyed/unkey#3785
File: go/apps/api/routes/v2_keys_reroll_key/401_test.go:52-61
Timestamp: 2025-08-14T16:25:48.167Z
Learning: User Flo4604 requested creation of a GitHub issue to track converting all test files to use table-driven test patterns as a broader codebase improvement, following the suggestion made during review of go/apps/api/routes/v2_keys_reroll_key/401_test.go.
Applied to files:
go/apps/api/routes/v2_ratelimit_delete_override/handler.go
🧬 Code graph analysis (2)
go/apps/api/routes/v2_ratelimit_limit/handler.go (9)
go/pkg/cache/scoped_key.go (1)
ScopedKey(44-57)go/pkg/db/retry.go (1)
WithRetry(28-67)go/pkg/db/ratelimit_namespace_find.sql_generated.go (1)
FindRatelimitNamespaceRow(37-45)go/internal/services/caches/op.go (1)
DefaultFindFirstOp(9-22)go/pkg/db/handle_err_no_rows.go (1)
IsNotFound(8-10)go/pkg/db/tx.go (1)
TxWithResult(113-147)go/pkg/uid/uid.go (2)
New(96-126)RatelimitNamespacePrefix(25-25)go/pkg/auditlog/events.go (1)
RatelimitNamespaceCreateEvent(26-26)go/pkg/ptr/deref.go (1)
SafeDeref(35-44)
go/apps/api/routes/v2_ratelimit_delete_override/handler.go (6)
go/pkg/cache/scoped_key.go (1)
ScopedKey(44-57)go/pkg/cache/interface.go (2)
Key(34-36)Null(42-42)go/pkg/db/retry.go (1)
WithRetry(28-67)go/pkg/db/ratelimit_namespace_find.sql_generated.go (1)
FindRatelimitNamespaceRow(37-45)go/internal/services/caches/op.go (1)
DefaultFindFirstOp(9-22)go/internal/services/caches/caches.go (1)
New(67-109)
🪛 LanguageTool
apps/docs/errors/unkey/data/ratelimit_namespace_gone.mdx
[grammar] ~5-~5: Use correct spacing
Context: ...limit Namespace Gone --- ## Description This error occurs when you attempt to us...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~7-~7: There might be a mistake here.
Context: ... error occurs when you attempt to use a ratelimit namespace that has been deleted. Once a...
(QB_NEW_EN_OTHER)
[grammar] ~7-~7: Use correct spacing
Context: ...e restored through the API or dashboard. ## Error Code `unkey/data/ratelimit_namesp...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~9-~9: Use correct spacing
Context: ...ugh the API or dashboard. ## Error Code unkey/data/ratelimit_namespace_gone ## HTTP Status Code 410 Gone ## Cause ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~13-~13: Use correct spacing
Context: ...mit_namespace_gone ## HTTP Status Code 410 Gone` ## Cause The ratelimit namespace you're tr...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~17-~17: Use correct spacing
Context: ...# HTTP Status Code 410 Gone ## Cause The ratelimit namespace you're trying to...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~19-~19: Use correct spacing
Context: ... available through the API or dashboard. ## Resolution Contact [support@unke...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~21-~21: Use correct spacing
Context: ...ugh the API or dashboard. ## Resolution Contact support@unkey.dev with your workspace ID and namespace name if you need this namespace restored. ## Prevention To avoid accidentally deleti...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~27-~27: Use correct spacing
Context: ...mespace restored. ## Prevention To avoid accidentally deleting namespace...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~29-~29: There might be a mistake here.
Context: ... avoid accidentally deleting namespaces: - Restrict namespace deletion via workspac...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...space deletion via workspace permissions - Carefully review namespace-deletion requ...
(QB_NEW_EN_OTHER)
[grammar] ~31-~31: There might be a mistake here.
Context: ...pace-deletion requests before confirming ## Related - [Ratelimit Namespace Not Foun...
(QB_NEW_EN_OTHER)
[grammar] ~33-~33: Use correct spacing
Context: ...n requests before confirming ## Related - [Ratelimit Namespace Not Found](/errors/u...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[uncategorized] ~36-~36: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: .../data/ratelimit_namespace_not_found) - [Rate Limiting Documentation](/ratelimiting/introducti...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
apps/docs/errors/user/bad_request/request_body_too_large.mdx
[grammar] ~80-~80: There might be a mistake here.
Context: ...](mailto:support@unkey.dev) and include: - What you're building - Why you need to s...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/docs/errors/user/bad_request/request_body_too_large.mdx (1)
80-80: Support address update looks correct.Switch to support@unkey.dev aligns with the thread about not supporting two domains.
go/demo_api/main.go (1)
620-620: LGTM: contact email switched to .dev.No behavioral impact; aligns with docs.
go/apps/api/routes/v2_ratelimit_delete_override/handler.go (1)
118-124: Should deleted namespaces return 410 Gone here as well?Limit handler uses Gone; this endpoint returns NotFound for deleted. If the intent is consistent semantics across namespace-aware endpoints, switch to Gone.
- if namespace.DeletedAtM.Valid { - return fault.New("namespace deleted", - fault.Code(codes.Data.RatelimitNamespace.NotFound.URN()), - fault.Internal("namespace deleted"), - fault.Public("This namespace does not exist."), - ) - } + if namespace.DeletedAtM.Valid { + return fault.New("namespace was deleted", + fault.Code(codes.Data.RatelimitNamespace.Gone.URN()), + fault.Public("This namespace has been deleted. Contact support to restore."), + ) + }go/apps/api/openapi/spec/paths/v2/ratelimit/limit/index.yaml (1)
93-93: LGTM: 200 description correctly directs clients tosuccessThis aligns with the behavior and reduces misuse of HTTP status for limit results.
3 participants
What does this PR do?
This creates a ratelimit namespace if it doesn't already exist IF only if the root key has permissions for it, otherwise an 403 will be thrown.
Also: If a namespace is soft deleted it throws a 410 error
Gonewith docs telling them to contact us so we can manually delete it.My plan was to add a
permanentflag to the delete namespace api which you can use to not-soft delete a namespace.Type of change
How should this be tested?
Ratelimit with a namespace that doesn't exist yet -> it should be created with an auditlog saying it got created
Ratelimit with a namsepace that doesn't exist yet but the key only has limit perms and can't create rl namespace's -> a 403 should be thrown
Ratelimit with a soft deleted namespace -> a 410 error should be thrown
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit