Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore!: deprecate reason for ArcjetAllowDecision #2715

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions decorate/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,9 @@ function extractReason(result: ArcjetRuleResult): ArcjetReason {
}

function isRateLimitReason(
reason: ArcjetReason,
reason?: ArcjetReason,
): reason is ArcjetRateLimitReason {
return reason.isRateLimit();
return typeof reason !== "undefined" && reason.isRateLimit();
}

function nearestLimit(
Expand Down
57 changes: 42 additions & 15 deletions examples/nextjs-bot-categories/app/api/arcjet/route.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import arcjet, { botCategories, detectBot } from "@arcjet/next";
import arcjet, { ArcjetRuleResult, botCategories, detectBot } from "@arcjet/next";
import { NextResponse } from "next/server";

const aj = arcjet({
Expand All @@ -23,6 +23,26 @@ const aj = arcjet({
],
});

function reduceAllowedBots(bots: string[], rule: ArcjetRuleResult) {
if (rule.reason.isBot()) {
return [...bots, ...rule.reason.allowed]
} else {
return bots;
}
}

function reduceDeniedBots(bots: string[], rule: ArcjetRuleResult) {
if (rule.reason.isBot()) {
return [...bots, ...rule.reason.denied]
} else {
return bots;
}
}

function checkSpoofed(rule: ArcjetRuleResult) {
return rule.reason.isBot() && rule.reason.isSpoofed()
}

export async function GET(req: Request) {
const decision = await aj.protect(req);

Expand All @@ -33,20 +53,27 @@ export async function GET(req: Request) {
)
}

const headers = new Headers();
if (decision.reason.isBot()) {
// WARNING: This is illustrative! Don't share this metadata with users;
// otherwise they may use it to subvert bot detection!
headers.set("X-Arcjet-Bot-Allowed", decision.reason.allowed.join(", "))
headers.set("X-Arcjet-Bot-Denied", decision.reason.denied.join(", "))

// We need to check that the bot is who they say they are.
if (decision.reason.isSpoofed()) {
return NextResponse.json(
{ error: "You are pretending to be a good bot!" },
{ status: 403, headers },
);
}
const allowedBots = decision.results.reduce<string[]>(reduceAllowedBots, []);

Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed you are still using map/filter in the other examples (for rate limit). We should keep it uniform. The reason to use a reduce is to avoid creating extra arrays, but the implementation of the reduction function is creating a new array on each iteration which is worse than a single filter then a map.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed this to a reduce. This removes checking that a limit isn't duplicated which is discussed in another comment though.

const deniedBots = decision.results.reduce<string[]>(reduceDeniedBots, []);

const isSpoofed = decision.results.some(checkSpoofed);

// WARNING: This is illustrative! Don't share this metadata with users;
// otherwise they may use it to subvert bot detection!
const headers = new Headers({
"X-Arcjet-Bot-Allowed": allowedBots.join(", "),
"X-Arcjet-Bot-Denied": deniedBots.join(", "),
});
headers.set("X-Arcjet-Bot-Allowed", allowedBots.join(", "))
headers.set("X-Arcjet-Bot-Denied", deniedBots.join(", "))

// We need to check that the bot is who they say they are.
if (isSpoofed) {
return NextResponse.json(
{ error: "You are pretending to be a good bot!" },
{ status: 403, headers },
);
}

if (decision.isDenied()) {
Expand Down
81 changes: 75 additions & 6 deletions examples/nextjs-clerk-rate-limit/app/api/arcjet/route.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import arcjet, { ArcjetDecision, tokenBucket, shield } from "@arcjet/next";
import arcjet, { ArcjetDecision, tokenBucket, shield, ArcjetRateLimitReason, ArcjetReason, ArcjetRuleResult } from "@arcjet/next";
import format from "@arcjet/sprintf";
import { NextRequest, NextResponse } from "next/server";
import { currentUser } from "@clerk/nextjs/server";
import ip from "@arcjet/ip";
Expand All @@ -16,6 +17,47 @@ const aj = arcjet({
],
});

function extractReason(result: ArcjetRuleResult): ArcjetReason {
return result.reason;
}

function isRateLimitReason(
reason?: ArcjetReason,
): reason is ArcjetRateLimitReason {
return typeof reason !== "undefined" && reason.isRateLimit();
}

function nearestLimit(
current: ArcjetRateLimitReason,
next: ArcjetRateLimitReason,
) {
if (current.remaining < next.remaining) {
return current;
}

if (current.remaining > next.remaining) {
return next;
}

// Reaching here means `remaining` is equal so prioritize closest reset
if (current.reset < next.reset) {
return current;
}

if (current.reset > next.reset) {
return next;
}

// Reaching here means that `remaining` and `reset` are equal, so prioritize
// the smallest `max`
if (current.max < next.max) {
return current;
}

// All else equal, just return the next item in the list
return next;
}

export async function GET(req: NextRequest) {
// Get the current user from Clerk
// See https://clerk.com/docs/references/nextjs/current-user
Expand Down Expand Up @@ -69,13 +111,40 @@ export async function GET(req: NextRequest) {
);
}
}

let reset: Date | undefined;
let remaining: number | undefined;

if (decision.reason.isRateLimit()) {
reset = decision.reason.resetTime;
remaining = decision.reason.remaining;
const rateLimitReasons = decision.results
.map(extractReason)
.filter(isRateLimitReason);

if (rateLimitReasons.length > 0) {
const policies = new Map<number, number>();
for (const reason of rateLimitReasons) {
if (policies.has(reason.max)) {
console.error(
"Invalid rate limit policy—two policies should not share the same limit",
);
}

if (
typeof reason.max !== "number" ||
typeof reason.window !== "number" ||
typeof reason.remaining !== "number" ||
typeof reason.reset !== "number"
) {
console.error(format("Invalid rate limit encountered: %o", reason));
}

policies.set(reason.max, reason.window);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Policies isn't doing anything? Did you want to break out of the logic or something?

Copy link
Contributor Author

@e-moran e-moran Jan 10, 2025

Choose a reason for hiding this comment

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

Its used to print an error if two rate limit policies share the same limit, like we do in decorate. The difference is that decorate returns an error in this case. We could respond with a 500 here to have the same behavior. What do you think?

Another option would be to detect invalid configs earlier, such as when initializing Arcjet. But that would be a much larger change.

const rl = rateLimitReasons.reduce(nearestLimit);

reset = rl.resetTime;
remaining = rl.remaining;
}

return NextResponse.json({ message: "Hello World", reset, remaining });}
return NextResponse.json({ message: "Hello World", reset, remaining });
}
22 changes: 22 additions & 0 deletions examples/nextjs-clerk-rate-limit/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion examples/nextjs-clerk-rate-limit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
},
"dependencies": {
"@arcjet/next": "file:../../arcjet-next",
"@arcjet/sprintf": "file:../../sprintf",
Copy link
Contributor

Choose a reason for hiding this comment

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

Might not be needed if you don't keep the policies stuff

"@clerk/nextjs": "^6.9.1",
"next": "15.1.0",
"react": "^19",
Expand All @@ -26,4 +27,4 @@
"tailwindcss": "^3.4.16",
"typescript": "^5"
}
}
}
84 changes: 79 additions & 5 deletions examples/nextjs-openai/app/api/chat/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
adding custom characteristics so you could use a user ID to track authenticated
users instead. See the `chat_userid` example for an example of this.
*/
import arcjet, { shield, tokenBucket } from "@arcjet/next";
import arcjet, { ArcjetRateLimitReason, ArcjetReason, ArcjetRuleResult, shield, tokenBucket } from "@arcjet/next";
import format from "@arcjet/sprintf";
import { openai } from '@ai-sdk/openai';
import { streamText } from 'ai';
import { promptTokensEstimate } from "openai-chat-tokens";
Expand All @@ -39,6 +40,47 @@ const aj = arcjet({
],
});

function extractReason(result: ArcjetRuleResult): ArcjetReason {
return result.reason;
}

function isRateLimitReason(
reason?: ArcjetReason,
): reason is ArcjetRateLimitReason {
return typeof reason !== "undefined" && reason.isRateLimit();
}

function nearestLimit(
current: ArcjetRateLimitReason,
next: ArcjetRateLimitReason,
) {
if (current.remaining < next.remaining) {
return current;
}

if (current.remaining > next.remaining) {
return next;
}

// Reaching here means `remaining` is equal so prioritize closest reset
if (current.reset < next.reset) {
return current;
}

if (current.reset > next.reset) {
return next;
}

// Reaching here means that `remaining` and `reset` are equal, so prioritize
// the smallest `max`
if (current.max < next.max) {
return current;
}

// All else equal, just return the next item in the list
return next;
}

// Allow streaming responses up to 30 seconds
export const maxDuration = 30;

Expand All @@ -48,7 +90,7 @@ export const runtime = "edge";
export async function POST(req: Request) {
const { messages } = await req.json();

// Estimate the number of tokens required to process the request
// Estimate the number of tokens required to process the request
const estimate = promptTokensEstimate({
messages,
});
Expand All @@ -59,8 +101,40 @@ export async function POST(req: Request) {
const decision = await aj.protect(req, { requested: estimate });
console.log("Arcjet decision", decision.conclusion);

if (decision.reason.isRateLimit()) {
console.log("Requests remaining", decision.reason.remaining);
const rateLimitReasons = decision.results
.map(extractReason)
.filter(isRateLimitReason);

let remaining: number | undefined;

if (rateLimitReasons.length > 0) {
const policies = new Map<number, number>();
for (const reason of rateLimitReasons) {
if (policies.has(reason.max)) {
console.error(
"Invalid rate limit policy—two policies should not share the same limit",
);
}

if (
typeof reason.max !== "number" ||
typeof reason.window !== "number" ||
typeof reason.remaining !== "number" ||
typeof reason.reset !== "number"
) {
console.error(format("Invalid rate limit encountered: %o", reason));
}

policies.set(reason.max, reason.window);
Copy link
Contributor

Choose a reason for hiding this comment

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

see above

}

const rl = rateLimitReasons.reduce(nearestLimit);

remaining = rl.remaining;
}

if (typeof remaining !== "undefined") {
console.log("Requests remaining", remaining);
}

// If the request is denied, return a 429
Expand All @@ -83,4 +157,4 @@ export async function POST(req: Request) {
});

return result.toDataStreamResponse();
}
}
Loading
Loading