fix(cloudflare): tag Turnstile missing widgets as NotFound#303
Conversation
| // Errors | ||
| // ============================================================================= | ||
|
|
||
| export class NotFound extends Schema.TaggedErrorClass<NotFound>()("NotFound", { | ||
| message: Schema.String, | ||
| }) {} | ||
| // Turnstile returns a bare 404 for missing widgets instead of Cloudflare's | ||
| // normal error envelope, so match the HTTP status directly. | ||
| T.applyErrorMatchers(NotFound, [{ status: 404 }]); |
There was a problem hiding this comment.
these are generated files do not modify them directly. add a patch and then rerun generate
There was a problem hiding this comment.
fixed: moved this into packages/cloudflare/patches/turnstile/{getWidget,deleteWidget}.json and reran bun scripts/generate.ts --service turnstile, so turnstile.ts is generated again.
There was a problem hiding this comment.
why are we making changes to this file?
There was a problem hiding this comment.
this change is needed because Turnstile returns bare/non-envelope 404 responses for missing widgets. the generated operation patch can now declare { status: 404 }, but api.ts has to try operation-specific matchers before falling back to generic CloudflareHttpError for those non-envelope responses. added comments and regression tests for both non-json and non-envelope cases.
agcty
force-pushed
the
codex/cloudflare-turnstile-not-found
branch
from
b1121fc to
f497a5d
Compare
| @@ -256,10 +256,55 @@ function findMatchingError( | |||
| return bestMatch; | |||
| } | |||
|
|
|||
| function matchOperationError( | |||
There was a problem hiding this comment.
Hmm, why is this necessary?
There was a problem hiding this comment.
it seems to be to handle when there is no envelope from cloudflare; I found it weird when i first looked at this, but it seems fine
There was a problem hiding this comment.
Added a // comment above matchOperationError explaining the Cloudflare no-envelope case. The generated Turnstile operation patch declares the 404 matcher, but api.ts has to try operation-specific matchers before falling back to CloudflareHttpError when Cloudflare returns a bare HTTP failure. Re-ran bun --filter @distilled.cloud/cloudflare check and bun run vitest run test/client-api.test.ts from packages/cloudflare.
agcty
force-pushed
the
codex/cloudflare-turnstile-not-found
branch
from
f497a5d to
6750db7
Compare
| if (!errors || errors.length === 0) return undefined; | ||
|
|
||
| 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); | ||
| } | ||
| } |
There was a problem hiding this comment.
We shouldn't do all this work on every request. Instead, it should create this map and return a function for matching, that way we cache the map in the closure once.
There was a problem hiding this comment.
addressed, lmk what you think
| status: number, | ||
| errorBody: unknown, | ||
| errors?: readonly ApiErrorClass[], | ||
| headers?: Record<string, string | undefined>, | ||
| ): Effect.Effect<never, unknown> => { | ||
| const matchOperationError = getOperationErrorMatcher(errors); |
There was a problem hiding this comment.
This is not caching anything. It bulds the matcher on every request still
| const cached = operationErrorMatcherCache.get(errors); | ||
| if (cached) return cached; |
There was a problem hiding this comment.
Using a global map is a massive code smell
There was a problem hiding this comment.
I looked into, i can see why AI is doing that. API.make's matchError function is limiting. We really need to rework this part of the codebase.
3 participants
Summary
NotFounderrorThis unblocks cleaning up alchemy-effect PR #370 so it can catch
NotFoundinstead of checking forCloudflareHttpError+ status 404.Verification
bun --filter @distilled.cloud/cloudflare test -- test/client-api.test.tsbun --filter @distilled.cloud/cloudflare typecheckbun --filter @distilled.cloud/cloudflare check