-
Notifications
You must be signed in to change notification settings - Fork 41
fix(cloudflare): tag Turnstile missing widgets as NotFound #303
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| { | ||
| "errors": { | ||
| "NotFound": [{ "status": 404 }] | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| { | ||
| "errors": { | ||
| "NotFound": [{ "status": 404 }] | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -220,6 +220,12 @@ function matcherSpecificity(matcher: ErrorMatcher): number { | |
| return score; | ||
| } | ||
|
|
||
| interface OperationErrorSchema { | ||
| schema: Schema.Top; | ||
| tag: string; | ||
| matchers: readonly ErrorMatcher[]; | ||
| } | ||
|
|
||
| interface MatchedError { | ||
| schema: Schema.Top; | ||
| tag: string; | ||
|
|
@@ -229,25 +235,24 @@ interface MatchedError { | |
| * Find matching error schema using annotations from the schema AST. | ||
| */ | ||
| function findMatchingError( | ||
| errorSchemas: Map<string, Schema.Top>, | ||
| errorSchemas: readonly OperationErrorSchema[], | ||
| code: number | undefined, | ||
| status: number, | ||
| message: string, | ||
| ): MatchedError | undefined { | ||
| let bestMatch: MatchedError | undefined; | ||
| let bestScore = 0; | ||
|
|
||
| for (const [name, schema] of errorSchemas) { | ||
| const ast = schema.ast; | ||
| const matchers = getErrorMatchers(ast); | ||
| if (!matchers || matchers.length === 0) continue; | ||
|
|
||
| for (const matcher of matchers) { | ||
| for (const errorSchema of errorSchemas) { | ||
| for (const matcher of errorSchema.matchers) { | ||
| if (matchesExpression(matcher, code, status, message)) { | ||
| const score = matcherSpecificity(matcher); | ||
| if (score > bestScore) { | ||
| bestScore = score; | ||
| bestMatch = { schema, tag: name }; | ||
| bestMatch = { | ||
| schema: errorSchema.schema, | ||
| tag: errorSchema.tag, | ||
| }; | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -256,22 +261,95 @@ function findMatchingError( | |
| return bestMatch; | ||
| } | ||
|
|
||
| type OperationErrorMatcher = ( | ||
| code: number | undefined, | ||
| status: number, | ||
| message: string, | ||
| ) => Effect.Effect<never, unknown> | undefined; | ||
|
|
||
| const noOperationErrorMatcher: OperationErrorMatcher = () => undefined; | ||
| const operationErrorMatcherCache = new WeakMap< | ||
| readonly ApiErrorClass[], | ||
| OperationErrorMatcher | ||
| >(); | ||
|
|
||
| // Some Cloudflare endpoints return bare HTTP failures instead of the standard | ||
| // `{ success: false, errors: [...] }` envelope. Generated operation schemas can | ||
| // still declare status-based error matchers, so try those before falling back | ||
| // to generic HTTP errors. | ||
| function getOperationErrorMatcher( | ||
| errors: readonly ApiErrorClass[] | undefined, | ||
| ): OperationErrorMatcher { | ||
| if (!errors || errors.length === 0) return noOperationErrorMatcher; | ||
|
|
||
| const cached = operationErrorMatcherCache.get(errors); | ||
| if (cached) return cached; | ||
|
Comment on lines
+285
to
+286
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using a global map is a massive code smell
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked into, i can see why AI is doing that. API.make's |
||
|
|
||
| const errorSchemas: OperationErrorSchema[] = []; | ||
| for (const errorSchema of errors) { | ||
| const schema = errorSchema as unknown as Schema.Top; | ||
| const identifier = extractTagFromAst(schema.ast); | ||
| const matchers = getErrorMatchers(schema.ast); | ||
| if (identifier && matchers && matchers.length > 0) { | ||
| errorSchemas.push({ schema, tag: identifier, matchers }); | ||
| } | ||
| } | ||
|
|
||
| const matcher: OperationErrorMatcher = (code, status, message) => { | ||
| const matched = findMatchingError(errorSchemas, code, status, message); | ||
| if (!matched) return undefined; | ||
|
|
||
| const errorData = { | ||
| _tag: matched.tag, | ||
| code: code ?? 0, | ||
| message, | ||
| }; | ||
| return Schema.decodeUnknownEffect(matched.schema)(errorData).pipe( | ||
| Effect.flatMap((decoded: unknown) => Effect.fail(decoded)), | ||
| Effect.catchIf( | ||
| (e: unknown) => | ||
| typeof e === "object" && | ||
| e !== null && | ||
| "_tag" in e && | ||
| (e as any)._tag === "SchemaError", | ||
| () => | ||
| Effect.fail( | ||
| new UnknownCloudflareError({ | ||
| code, | ||
| message, | ||
| }), | ||
| ), | ||
| ), | ||
| ) as Effect.Effect<never, unknown>; | ||
| }; | ||
| operationErrorMatcherCache.set(errors, matcher); | ||
| return matcher; | ||
| } | ||
|
|
||
| /** | ||
| * Match a Cloudflare API error response using per-operation error schemas. | ||
| */ | ||
| const matchError = ( | ||
| export const matchCloudflareError = ( | ||
| status: number, | ||
| errorBody: unknown, | ||
| errors?: readonly ApiErrorClass[], | ||
| headers?: Record<string, string | undefined>, | ||
| ): Effect.Effect<never, unknown> => { | ||
| const matchOperationError = getOperationErrorMatcher(errors); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not caching anything. It bulds the matcher on every request still |
||
|
|
||
| // Handle non-JSON error responses (e.g., HTML from malformed URLs, 520 pages) | ||
| const isNonJsonError = | ||
| typeof errorBody === "object" && | ||
| errorBody !== null && | ||
| "_nonJsonError" in errorBody; | ||
| if (isNonJsonError) { | ||
| const message = String((errorBody as any).body); | ||
| // Some Cloudflare endpoints return bare HTTP errors without the standard | ||
| // `{ success: false, errors: [...] }` envelope. Give operation-specific | ||
| // status matchers a chance before falling back to generic HTTP errors. | ||
| const matched = matchOperationError(undefined, status, message); | ||
| if (matched) return matched; | ||
|
|
||
| // For 5xx errors, return a properly categorized error so retries work | ||
| if (status >= 500) { | ||
| return Effect.fail(httpStatusError(status, message, headers)); | ||
|
|
@@ -316,6 +394,12 @@ const matchError = ( | |
| // For 5xx errors, return a properly categorized error so retries work | ||
| const bodyStr = | ||
| typeof errorBody === "string" ? errorBody : JSON.stringify(errorBody); | ||
| // Preserve per-operation error typing for endpoints whose error responses | ||
| // are JSON but not Cloudflare envelopes, such as Turnstile's missing-widget | ||
| // 404 response. | ||
| const matched = matchOperationError(undefined, status, bodyStr); | ||
| if (matched) return matched; | ||
|
|
||
| if (status >= 500) { | ||
| return Effect.fail(httpStatusError(status, bodyStr, headers)); | ||
| } | ||
|
|
@@ -329,51 +413,8 @@ const matchError = ( | |
| ); | ||
| } | ||
|
|
||
| // Build error schema map from the per-operation errors | ||
| if (errors && errors.length > 0) { | ||
| const errorSchemas = new Map<string, Schema.Top>(); | ||
| for (const errorSchema of errors) { | ||
| const identifier = extractTagFromAst( | ||
| (errorSchema as unknown as Schema.Top).ast, | ||
| ); | ||
| if (identifier) { | ||
| errorSchemas.set(identifier, errorSchema as unknown as Schema.Top); | ||
| } | ||
| } | ||
|
|
||
| const matched = findMatchingError( | ||
| errorSchemas, | ||
| errorCode, | ||
| status, | ||
| errorMessage, | ||
| ); | ||
|
|
||
| if (matched) { | ||
| // Decode using the schema - properly instantiates TaggedError classes | ||
| const errorData = { | ||
| _tag: matched.tag, | ||
| code: errorCode ?? 0, | ||
| message: errorMessage, | ||
| }; | ||
| return Schema.decodeUnknownEffect(matched.schema)(errorData).pipe( | ||
| Effect.flatMap((decoded: unknown) => Effect.fail(decoded)), | ||
| Effect.catchIf( | ||
| (e: unknown) => | ||
| typeof e === "object" && | ||
| e !== null && | ||
| "_tag" in e && | ||
| (e as any)._tag === "SchemaError", | ||
| () => | ||
| Effect.fail( | ||
| new UnknownCloudflareError({ | ||
| code: errorCode, | ||
| message: errorMessage, | ||
| }), | ||
| ), | ||
| ), | ||
| ) as Effect.Effect<never, unknown>; | ||
| } | ||
| } | ||
| const matched = matchOperationError(errorCode, status, errorMessage); | ||
| if (matched) return matched; | ||
|
|
||
| // Check global error codes before falling through to unknown | ||
| if (errorCode !== undefined && errorCode in GLOBAL_ERROR_CODE_MAP) { | ||
|
|
@@ -462,7 +503,7 @@ const _API = makeAPI<Credentials>({ | |
| credentials: Credentials as any, | ||
| getBaseUrl: (creds: any) => creds.apiBaseUrl, | ||
| getAuthHeaders: formatHeaders as any, | ||
| matchError, | ||
| matchError: matchCloudflareError, | ||
| ParseError: CloudflareDecodeError as any, | ||
| transformRequestParts: ({ pathTemplate, parts }) => | ||
| transformCloudflareRequestParts({ pathTemplate, parts }), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,16 @@ import * as T from "../traits.ts"; | |
| import type { Credentials } from "../credentials.ts"; | ||
| import { type DefaultErrors } from "../errors.ts"; | ||
|
|
||
| // ============================================================================= | ||
| // Errors | ||
| // ============================================================================= | ||
|
|
||
| export class NotFound extends Schema.TaggedErrorClass<NotFound>()("NotFound", { | ||
| code: Schema.Number, | ||
| message: Schema.String, | ||
| }) {} | ||
| T.applyErrorMatchers(NotFound, [{ status: 404 }]); | ||
|
Comment on lines
+16
to
+23
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these are generated files do not modify them directly. add a patch and then rerun generate
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed: moved this into |
||
|
|
||
| // ============================================================================= | ||
| // SecretWidget | ||
| // ============================================================================= | ||
|
|
@@ -201,7 +211,7 @@ export const GetWidgetResponse = /*@__PURE__*/ /*#__PURE__*/ Schema.Struct({ | |
| T.ResponsePath("result"), | ||
| ) as unknown as Schema.Schema<GetWidgetResponse>; | ||
|
|
||
| export type GetWidgetError = DefaultErrors; | ||
| export type GetWidgetError = DefaultErrors | NotFound; | ||
|
|
||
| export const getWidget: API.OperationMethod< | ||
| GetWidgetRequest, | ||
|
|
@@ -211,7 +221,7 @@ export const getWidget: API.OperationMethod< | |
| > = /*@__PURE__*/ /*#__PURE__*/ API.make(() => ({ | ||
| input: GetWidgetRequest, | ||
| output: GetWidgetResponse, | ||
| errors: [], | ||
| errors: [NotFound], | ||
| })); | ||
|
|
||
| export interface ListWidgetsRequest { | ||
|
|
@@ -690,7 +700,7 @@ export const DeleteWidgetResponse = /*@__PURE__*/ /*#__PURE__*/ Schema.Struct({ | |
| T.ResponsePath("result"), | ||
| ) as unknown as Schema.Schema<DeleteWidgetResponse>; | ||
|
|
||
| export type DeleteWidgetError = DefaultErrors; | ||
| export type DeleteWidgetError = DefaultErrors | NotFound; | ||
|
|
||
| export const deleteWidget: API.OperationMethod< | ||
| DeleteWidgetRequest, | ||
|
|
@@ -700,5 +710,5 @@ export const deleteWidget: API.OperationMethod< | |
| > = /*@__PURE__*/ /*#__PURE__*/ API.make(() => ({ | ||
| input: DeleteWidgetRequest, | ||
| output: DeleteWidgetResponse, | ||
| errors: [], | ||
| errors: [NotFound], | ||
| })); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we making changes to this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change is needed because Turnstile returns bare/non-envelope 404 responses for missing widgets. the generated operation patch can now declare
{ status: 404 }, butapi.tshas to try operation-specific matchers before falling back to genericCloudflareHttpErrorfor those non-envelope responses. added comments and regression tests for both non-json and non-envelope cases.