Skip to content

Commit

Permalink
fix: retry on any error with disabled cache
Browse files Browse the repository at this point in the history
  • Loading branch information
chronark committed Oct 17, 2024
1 parent 41571cc commit 62df070
Show file tree
Hide file tree
Showing 7 changed files with 571 additions and 197 deletions.
4 changes: 2 additions & 2 deletions apps/api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"@vitest/ui": "^1.6.0",
"typescript": "^5.5.3",
"vitest": "^1.6.0",
"wrangler": "^3.62.0"
"wrangler": "^3.80.5"
},
"dependencies": {
"@axiomhq/js": "1.0.0-rc.2",
Expand All @@ -27,6 +27,7 @@
"@hono/zod-validator": "^0.2.1",
"@planetscale/database": "^1.16.0",
"@unkey/cache": "workspace:^",
"@unkey/clickhouse-zod": "workspace:^",
"@unkey/db": "workspace:^",
"@unkey/encryption": "workspace:^",
"@unkey/error": "workspace:^",
Expand All @@ -36,7 +37,6 @@
"@unkey/logs": "workspace:^",
"@unkey/metrics": "workspace:^",
"@unkey/rbac": "workspace:^",
"@unkey/clickhouse-zod": "workspace:^",
"@unkey/schema": "workspace:^",
"@unkey/worker-logging": "workspace:^",
"hono": "^4.6.3",
Expand Down
287 changes: 152 additions & 135 deletions apps/api/src/pkg/keys/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { Context } from "@/pkg/hono/app";
import type { Metrics } from "@/pkg/metrics";
import type { RateLimiter } from "@/pkg/ratelimit";
import type { UsageLimiter } from "@/pkg/usagelimit";
import { BaseError, Err, FetchError, Ok, type Result, SchemaError } from "@unkey/error";
import { BaseError, Err, FetchError, Ok, type Result, SchemaError, wrap } from "@unkey/error";
import { sha256 } from "@unkey/hash";
import type { PermissionQuery, RBAC } from "@unkey/rbac";
import type { Logger } from "@unkey/worker-logging";
Expand Down Expand Up @@ -142,19 +142,13 @@ export class KeyService {
>
> {
try {
let res = await this._verifyKey(c, req);
let remainingRetries = 3;
while (res.err && remainingRetries > 0) {
remainingRetries--;
this.logger.warn("retrying verification", {
remainingRetries,
previousError: res.err.message,
const res = await this._verifyKey(c, req).catch(async (err) => {
this.logger.error("verify error, retrying without cache", {
error: err.message,
});

await this.cache.keyByHash.remove(await this.hash(req.key));
res = await this._verifyKey(c, req);
}

return await this._verifyKey(c, req, { skipCache: true });
});
if (res.err) {
this.metrics.emit({
metric: "metric.key.verification",
Expand Down Expand Up @@ -188,6 +182,118 @@ export class KeyService {
}
}

private async getData(hash: string) {
const dbStart = performance.now();
const query = this.db.readonly.query.keys.findFirst({
where: (table, { and, eq, isNull }) => and(eq(table.hash, hash), isNull(table.deletedAt)),
with: {
encrypted: true,
workspace: {
columns: {
id: true,
enabled: true,
},
},
forWorkspace: {
columns: {
id: true,
enabled: true,
},
},
roles: {
with: {
role: {
with: {
permissions: {
with: {
permission: true,
},
},
},
},
},
},
permissions: {
with: {
permission: true,
},
},
keyAuth: {
with: {
api: true,
},
},
ratelimits: true,
identity: {
with: {
ratelimits: true,
},
},
},
});

const dbRes = await query;
this.metrics.emit({
metric: "metric.db.read",
query: "getKeyAndApiByHash",
latency: performance.now() - dbStart,
});

if (!dbRes?.keyAuth?.api) {
return null;
}
if (dbRes.keyAuth.deletedAt) {
return null;
}
if (dbRes.keyAuth.api.deletedAt) {
return null;
}

/**
* Createa a unique set of all permissions, whether they're attached directly or connected
* through a role.
*/
const permissions = new Set<string>([
...dbRes.permissions.map((p) => p.permission.name),
...dbRes.roles.flatMap((r) => r.role.permissions.map((p) => p.permission.name)),
]);

/**
* Merge ratelimits from the identity and the key
* Key limits take pecedence
*/
const ratelimits: { [name: string]: Pick<Ratelimit, "name" | "limit" | "duration"> } = {};

if (
dbRes.ratelimitAsync !== null &&
dbRes.ratelimitDuration !== null &&
dbRes.ratelimitLimit !== null
) {
ratelimits.default = {
name: "default",
limit: dbRes.ratelimitLimit,
duration: dbRes.ratelimitDuration,
};
}
for (const rl of dbRes.identity?.ratelimits ?? []) {
ratelimits[rl.name] = rl;
}
for (const rl of dbRes.ratelimits ?? []) {
ratelimits[rl.name] = rl;
}

return {
workspace: dbRes.workspace,
forWorkspace: dbRes.forWorkspace,
key: dbRes,
identity: dbRes.identity,
api: dbRes.keyAuth.api,
permissions: Array.from(permissions.values()),
roles: dbRes.roles.map((r) => r.role.name),
ratelimits,
};
}

private async hash(key: string): Promise<string> {
const cached = this.hashCache.get(key);
if (cached) {
Expand All @@ -209,137 +315,48 @@ export class KeyService {
ratelimit?: { cost?: number };
ratelimits?: Array<Omit<RatelimitRequest, "identity">>;
},
opts?: {
skipCache?: boolean;
},
): Promise<
Result<
VerifyKeyResult,
FetchError | SchemaError | DisabledWorkspaceError | MissingRatelimitError
>
> {
const keyHash = await this.hash(req.key);
const { val: data, err } = await retry(
3,
async () =>
this.cache.keyByHash.swr(keyHash, async (hash) => {
const dbStart = performance.now();
const query = this.db.readonly.query.keys.findFirst({
where: (table, { and, eq, isNull }) =>
and(eq(table.hash, hash), isNull(table.deletedAt)),
with: {
encrypted: true,
workspace: {
columns: {
id: true,
enabled: true,
},
},
forWorkspace: {
columns: {
id: true,
enabled: true,
},
},
roles: {
with: {
role: {
with: {
permissions: {
with: {
permission: true,
},
},
},
},
},
},
permissions: {
with: {
permission: true,
},
},
keyAuth: {
with: {
api: true,
},
},
ratelimits: true,
identity: {
with: {
ratelimits: true,
},

if (opts?.skipCache) {
this.logger.info("skipping cache", {
keyHash,
});
}
const { val: data, err } = opts?.skipCache
? await wrap(
this.getData(keyHash),
(err) =>
new FetchError({
message: "unable to query db",
retry: false,
context: {
error: err.message,
url: "",
method: "",
keyHash,
},
},
});

const dbRes = await query;
this.metrics.emit({
metric: "metric.db.read",
query: "getKeyAndApiByHash",
latency: performance.now() - dbStart,
dbRes: JSON.stringify(dbRes),
sql: query.toSQL().sql,
});
if (!dbRes?.keyAuth?.api) {
return null;
}
if (dbRes.keyAuth.deletedAt) {
return null;
}
if (dbRes.keyAuth.api.deletedAt) {
return null;
}

/**
* Createa a unique set of all permissions, whether they're attached directly or connected
* through a role.
*/
const permissions = new Set<string>([
...dbRes.permissions.map((p) => p.permission.name),
...dbRes.roles.flatMap((r) => r.role.permissions.map((p) => p.permission.name)),
]);

/**
* Merge ratelimits from the identity and the key
* Key limits take pecedence
*/
const ratelimits: { [name: string]: Pick<Ratelimit, "name" | "limit" | "duration"> } = {};

if (
dbRes.ratelimitAsync !== null &&
dbRes.ratelimitDuration !== null &&
dbRes.ratelimitLimit !== null
) {
ratelimits.default = {
name: "default",
limit: dbRes.ratelimitLimit,
duration: dbRes.ratelimitDuration,
};
}
for (const rl of dbRes.identity?.ratelimits ?? []) {
ratelimits[rl.name] = rl;
}
for (const rl of dbRes.ratelimits ?? []) {
ratelimits[rl.name] = rl;
}

return {
workspace: dbRes.workspace,
forWorkspace: dbRes.forWorkspace,
key: dbRes,
identity: dbRes.identity,
api: dbRes.keyAuth.api,
permissions: Array.from(permissions.values()),
roles: dbRes.roles.map((r) => r.role.name),
ratelimits,
};
}),
(attempt, err) => {
this.logger.warn("Failed to fetch key data, retrying...", {
hash: keyHash,
attempt,
error: err.message,
});
},
);
}),
)
: await retry(
3,
async () => this.cache.keyByHash.swr(keyHash, (h) => this.getData(h)),
(attempt, err) => {
this.logger.warn("Failed to fetch key data, retrying...", {
hash: keyHash,
attempt,
error: err.message,
});
},
);

if (err) {
this.logger.error(err.message, {
Expand Down
5 changes: 5 additions & 0 deletions apps/api/wrangler.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ compatibility_date = "2024-01-01"

compatibility_flags = ["nodejs_compat"]

[observability]
enabled = true

## these are in local dev
[durable_objects]
Expand Down Expand Up @@ -99,3 +101,6 @@ consumers = [
{ queue = "key-migrations-production", max_batch_size = 10, max_retries = 10, dead_letter_queue = "key-migrations-production-dlq" },
{ queue = "key-migrations-production-dlq", max_batch_size = 10, max_retries = 10 },
]

[env.production.observability]
enabled = true
2 changes: 1 addition & 1 deletion apps/logdrain/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@
"devDependencies": {
"@cloudflare/workers-types": "4.20240603.0",
"typescript": "^5.5.3",
"wrangler": "^3.62.0"
"wrangler": "^3.80.5"
}
}
2 changes: 1 addition & 1 deletion apps/semantic-cache/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"@unkey/tsconfig": "workspace:^",
"dotenv": "^16.4.5",
"typescript": "^5.5.3",
"wrangler": "^3.62.0"
"wrangler": "^3.80.5"
},
"dependencies": {
"@chronark/zod-bird": "^0.3.9",
Expand Down
2 changes: 2 additions & 0 deletions packages/api/src/openapi.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,8 @@ export interface components {
* - INSUFFICIENT_PERMISSIONS: you do not have the required permissions to perform this action
* - EXPIRED: The key was only valid for a certain time and has expired.
*
* These are validation codes, the HTTP status will be 200.
*
* @enum {string}
*/
code:
Expand Down
Loading

0 comments on commit 62df070

Please sign in to comment.