chore: small fixes for ensapi and openapi doc generation#1872
chore: small fixes for ensapi and openapi doc generation#1872
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
|
🦋 Changeset detectedLatest commit: d17133e The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdded OpenAPI examples and richer response/content schemas; introduced serialized registrar-action schemas and examples; renamed a request selection param ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR makes a focused set of quality-of-life improvements to the ENSApi and its OpenAPI documentation: it fixes a bug where the Key changes:
Confidence Score: 5/5Safe to merge; all remaining findings are P2 style/documentation suggestions that do not block correctness. The bug fix (app.onError), the parameter rename, and the serialized-schema additions are all logically correct and well-tested. The three P2 findings (typo in error message, changeset bump level, and OpenAPI boolean-vs-string annotation) are non-blocking quality items. No logic bugs or data-integrity issues were found. packages/ensdb-sdk/src/client/zod-schemas/ensdb-config.ts (typo) and apps/ensapi/src/app.ts (error message exposure) deserve a quick look before merge. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[HTTP Request] --> B{Route Handler}
B -->|success| C[serializeRegistrarActionsResponse]
C --> D[JSON Response 200]
B -->|ZodError / SchemaError thrown| E[app.onError]
B -->|Other error thrown| E
E --> F[errorResponse ctx error]
F -->|instanceof ZodError| G[400 Invalid Input + details]
F -->|instanceof SchemaError| G
F -->|instanceof Error| H[500 error.message]
F -->|unknown| I[500 Internal Server Error]
subgraph Deserialization [Client SDK: deserializeRegistrarActionsResponse]
J[Wire JSON] --> K[makeSerializedRegistrarActionsResponseSchema validates string amounts]
K -->|error| L[throw Error]
K -->|ok| M[makeRegistrarActionsResponseSchema transforms strings to BigInt]
M -->|error| N[throw Error]
M -->|ok| O[RegistrarActionsResponse domain type]
end
Reviews (1): Last reviewed commit: "add env names" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const rawSelectionParams = z.object({ | ||
| name: z.string().optional(), | ||
| addresses: z.string().optional(), | ||
| texts: z.string().optional(), | ||
| nameRecord: z | ||
| .string() | ||
| .optional() | ||
| .openapi({ | ||
| type: "boolean", | ||
| description: "Whether to include the ENS name record in the response.", | ||
| }), | ||
| addresses: z |
There was a problem hiding this comment.
type: "boolean" on a z.string() field
rawSelectionParams.nameRecord is declared as z.string() but then annotated with .openapi({ type: "boolean" }). This tells OpenAPI code generators and consumers that the parameter is a JSON boolean, yet over HTTP it is still a plain query-string value ("true" / "false"). Some clients may therefore attempt to serialise it as a literal true (no quotes), which is invalid for a query param.
A more accurate OpenAPI annotation would be:
| const rawSelectionParams = z.object({ | |
| name: z.string().optional(), | |
| addresses: z.string().optional(), | |
| texts: z.string().optional(), | |
| nameRecord: z | |
| .string() | |
| .optional() | |
| .openapi({ | |
| type: "boolean", | |
| description: "Whether to include the ENS name record in the response.", | |
| }), | |
| addresses: z | |
| nameRecord: z | |
| .string() | |
| .optional() | |
| .openapi({ | |
| type: "string", | |
| enum: ["true", "false"], | |
| description: "Whether to include the ENS name record in the response.", | |
| }), |
Alternatively, simply reuse boolstring (which already carries the correct .openapi({ type: "boolean" }) hint that @hono/zod-openapi understands from a string-with-coercion context) to stay consistent with the addresses / texts siblings.
| * The start index of the records on the page (0-indexed) | ||
| */ | ||
| startIndex: undefined; | ||
| startIndex?: undefined; |
There was a problem hiding this comment.
cannot generate openapi without it lol
| totalPages: z.literal(1), | ||
| hasNext: z.literal(false), | ||
| hasPrev: z.literal(false), | ||
| startIndex: z.undefined(), |
There was a problem hiding this comment.
cannot generate openapi doc without it lol. I dont know why without examples it works... js is mystery
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensapi/src/handlers/api/explore/registrar-actions-api.routes.ts`:
- Around line 103-113: The 200 response blocks currently use
makeSerializedRegistrarActionsResponseSchema(...) which produces both success
and error variants; replace those 200 blocks with the OK-only serialized schema
(the project’s "registrar actions OK" schema factory / serializer) and keep the
error shape only on the 500 responses; ensure the example
(registrarActionsResponseOkExample) is still attached to the OK-only schema so
the 200 response describes only the success payload and the 500 response
continues to reference the error schema.
In `@apps/ensapi/src/lib/handlers/params.schema.ts`:
- Around line 72-78: The schema change removed the public query key "name"
causing existing ?name=true callers to be dropped; restore backward
compatibility by accepting "name" as an alias to "nameRecord": add an optional
schema entry for the "name" query (same shape/docs as nameRecord) and update the
request-mapping logic that currently reads/sets nameRecord so it also checks for
and maps the legacy "name" key into nameRecord before validation/stripping
(referencing the nameRecord schema entry and the mapping code that assigns
nameRecord).
In `@packages/ensdb-sdk/src/client/zod-schemas/ensdb-config.ts`:
- Around line 4-7: The error message on the EnsDbUrlSchema z.string validator
contains a typo ("variavle"); update the error text for EnsDbUrlSchema to use
the correct spelling "variable" and ensure the message matches the wording used
elsewhere (e.g., other ENV error messages) so it's consistent across schemas.
In `@packages/ensnode-sdk/src/registrars/zod-schemas.ts`:
- Around line 227-245: Duplicate field definitions for serialized registrar
actions should be extracted into a single shared shape builder to avoid schema
drift: create a helper (e.g., makeSerializedRegistrarActionBaseShape or
makeBaseActionShape) that returns the common z.object shape including id
(EventIdSchema), incrementalDuration, registrant (makeLowercaseAddressSchema),
registrationLifecycle, referral (makeRegistrarActionReferralSchema), block
(makeBlockRefSchema), transactionHash (makeTransactionHashSchema), and eventIds
(EventIdsSchema) and apply invariant_eventIdsInitialElementIsTheActionId to that
shape; then update the existing makeBaseSerializedRegistrarActionSchema and the
other duplicated schema to compose that base shape and only add the differing
pricing field (using makeSerializedRegistrarActionPricingSchema) so all common
invariants are defined once.
- Around line 103-117: The serialized registrar action pricing schema
(makeSerializedRegistrarActionPricingSchema) currently accepts objects where
baseCost, premium, and total are individually valid but not checked for
consistency; update makeSerializedRegistrarActionPricingSchema to add the same
validation parity as the domain pricing schema by adding a
refinement/superRefine that, for the non-null object variant produced from
makeSerializedPriceEthSchema, computes baseCost + premium and asserts it equals
total (and still allows the null-object variant), so inconsistent serialized
payloads are rejected early.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fdd3c9b5-dea4-4106-9a75-97cdf1b0350e
📒 Files selected for processing (26)
.changeset/nice-foxes-cheat.md.changeset/smooth-foxes-boil.mdapps/ensapi/src/app.tsapps/ensapi/src/handlers/api/explore/name-tokens-api.routes.tsapps/ensapi/src/handlers/api/explore/registrar-actions-api.routes.tsapps/ensapi/src/handlers/api/explore/registrar-actions-api.tsapps/ensapi/src/handlers/api/meta/realtime-api.routes.tsapps/ensapi/src/handlers/api/meta/status-api.routes.tsapps/ensapi/src/handlers/api/resolution/resolution-api.routes.tsapps/ensapi/src/lib/handlers/params.schema.test.tsapps/ensapi/src/lib/handlers/params.schema.tsdocs/docs.ensnode.io/ensapi-openapi.jsonpackages/ensdb-sdk/src/client/zod-schemas/ensdb-config.tspackages/ensnode-sdk/src/ensapi/api/name-tokens/examples.tspackages/ensnode-sdk/src/ensapi/api/name-tokens/zod-schemas.test.tspackages/ensnode-sdk/src/ensapi/api/registrar-actions/deserialize.tspackages/ensnode-sdk/src/ensapi/api/registrar-actions/examples.tspackages/ensnode-sdk/src/ensapi/api/registrar-actions/serialize.tspackages/ensnode-sdk/src/ensapi/api/registrar-actions/zod-schemas.test.tspackages/ensnode-sdk/src/ensapi/api/registrar-actions/zod-schemas.tspackages/ensnode-sdk/src/ensapi/api/resolution/examples.tspackages/ensnode-sdk/src/ensapi/api/resolution/zod-schemas.test.tspackages/ensnode-sdk/src/ensapi/api/shared/pagination/response.tspackages/ensnode-sdk/src/ensapi/api/shared/pagination/zod-schemas.tspackages/ensnode-sdk/src/internal.tspackages/ensnode-sdk/src/registrars/zod-schemas.ts
💤 Files with no reviewable changes (3)
- apps/ensapi/src/handlers/api/meta/status-api.routes.ts
- packages/ensnode-sdk/src/ensapi/api/shared/pagination/zod-schemas.ts
- apps/ensapi/src/handlers/api/meta/realtime-api.routes.ts
| nameRecord: z | ||
| .string() | ||
| .optional() | ||
| .openapi({ | ||
| type: "boolean", | ||
| description: "Whether to include the ENS name record in the response.", | ||
| }), |
There was a problem hiding this comment.
Preserve name as a backward-compatible alias.
Line 72 removes the existing public name query key, and Line 101 now maps only nameRecord. Since this object schema strips unknown keys, existing ?name=true callers will silently lose that selection and can even fall into "Selection cannot be empty." Keep accepting name at runtime for now, even if only nameRecord stays documented.
Also applies to: 95-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ensapi/src/lib/handlers/params.schema.ts` around lines 72 - 78, The
schema change removed the public query key "name" causing existing ?name=true
callers to be dropped; restore backward compatibility by accepting "name" as an
alias to "nameRecord": add an optional schema entry for the "name" query (same
shape/docs as nameRecord) and update the request-mapping logic that currently
reads/sets nameRecord so it also checks for and maps the legacy "name" key into
nameRecord before validation/stripping (referencing the nameRecord schema entry
and the mapping code that assigns nameRecord).
There was a problem hiding this comment.
name record is quite niche, so we can skip it I guess?
There was a problem hiding this comment.
@sevenzing, understood! If the name record selection is niche and the API is not yet widely consumed with ?name=true, skipping the alias is a reasonable call. Just worth noting in release notes or a migration guide if any external consumers do rely on it. Thanks for the context!
(╯°□°）╯︵ ┻━┻
🧠 Learnings used
Learnt from: tk-o
Repo: namehash/ensnode PR: 1671
File: packages/ensnode-sdk/src/ensapi/api/indexing-status/zod-schemas.ts:64-68
Timestamp: 2026-03-02T17:40:32.679Z
Learning: In `packages/ensnode-sdk/src/ensapi/api/indexing-status/zod-schemas.ts`, the serialized Ok response schema (`makeSerializedEnsApiIndexingStatusResponseOkSchema`) intentionally uses `z.object` instead of `z.strictObject` to allow extra fields, while non-serialized schemas use `z.strictObject`. This design choice is deliberate for forward compatibility.
Learnt from: tk-o
Repo: namehash/ensnode PR: 1639
File: packages/ensnode-sdk/src/ensapi/api/indexing-status/zod-schemas.ts:21-76
Timestamp: 2026-02-16T17:53:46.139Z
Learning: In the ENSNode SDK (`packages/ensnode-sdk`), schema builder functions exported from `zod-schemas.ts` files (e.g., `makeEnsApiIndexingStatusResponseSchema`) are considered internal API, not public API. These can have breaking changes without requiring deprecated aliases, even when exported via the `internal` entry point.
Learnt from: Goader
Repo: namehash/ensnode PR: 1732
File: packages/ens-referrals/src/v1/api/serialized-types.ts:43-47
Timestamp: 2026-03-06T23:35:37.231Z
Learning: In `packages/ens-referrals`, the team is not concerned with preserving backward-compatibility aliases for removed public TypeScript types (e.g., SerializedReferrerEditionMetricsRanked, SerializedReferrerEditionMetricsUnranked) during active development. Breaking changes to exported types in this package are acceptable and do not require deprecated aliases.
Learnt from: CR
Repo: namehash/ensnode PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-04T02:29:09.908Z
Learning: Applies to apps/ensapi/src/**/*.ts : Use Zod for validation in API requests with the Hono validation middleware (validate() from apps/ensapi/src/lib/handlers/validate.ts); do not manually call zod.parse/safeParse in route handlers for request body/params/query
Learnt from: tk-o
Repo: namehash/ensnode PR: 1615
File: packages/ensnode-sdk/src/api/indexing-status/deserialize.ts:38-40
Timestamp: 2026-02-07T12:22:32.900Z
Learning: In `packages/ensnode-sdk/src/api/indexing-status/deserialize.ts`, the pattern of using `z.preprocess()` with `buildUnvalidatedIndexingStatusResponse` (which returns `unknown`) is intentional. This enforces a "parse serialized → preprocess to unvalidated → validate final schema" flow, where `z.preprocess` is semantically correct because it runs before final validation. Using `.transform()` would be incorrect here as it runs after parsing and receives a typed input.
Learnt from: notrab
Repo: namehash/ensnode PR: 1631
File: apps/ensapi/src/handlers/ensnode-api.ts:23-27
Timestamp: 2026-02-18T16:11:00.691Z
Learning: Dynamic imports inside request handlers can be acceptable if used judiciously, but they introduce asynchronous loading and potential variability in startup latency. This pattern should be reviewed on a per-file basis: prefer top-level imports for commonly used modules and reserve dynamic import for conditional loading or rare code paths. If you continue to use dynamic imports in ensnode-api.ts, ensure you profile startup and per-request latency, verify error handling for promise rejections, and document the reasoning for caching behavior.
Learnt from: Goader
Repo: namehash/ensnode PR: 1663
File: packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts:74-96
Timestamp: 2026-02-24T15:53:06.633Z
Learning: In TypeScript code reviews, prefer placing invariants on type aliases only when the invariant is context-independent or reused across multiple fields. If an invariant depends on surrounding rules or object semantics (e.g., field-specific metrics), keep the invariant as a field JSDoc instead. This guideline applies to TS files broadly (e.g., the repo's v1/award-models and similar modules).
There was a problem hiding this comment.
Yes, we should deliberately make a breaking API change in this PR and not make this change backwards compatible.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| const makeSerializedRegistrarActionPricingSchema = ( | ||
| valueLabel: string = "Serialized Registrar Action Pricing", | ||
| ) => | ||
| z.union([ | ||
| z.object({ | ||
| baseCost: makeSerializedPriceEthSchema(`${valueLabel} Base Cost`), | ||
| premium: makeSerializedPriceEthSchema(`${valueLabel} Premium`), | ||
| total: makeSerializedPriceEthSchema(`${valueLabel} Total`), | ||
| }), | ||
| z.object({ | ||
| baseCost: z.null(), | ||
| premium: z.null(), | ||
| total: z.null(), | ||
| }), | ||
| ]); |
There was a problem hiding this comment.
Add serialized pricing total validation parity.
Line [107]-Line [111] validate shape, but unlike the domain pricing schema (Line [77]), this path does not enforce total = baseCost + premium. That lets inconsistent serialized payloads pass this schema and fail later.
Suggested fix
+function invariant_serializedRegistrarActionPricingTotalIsSumOfBaseCostAndPremium(
+ ctx: ParsePayload<{
+ baseCost: { amount: string; currency: "ETH" };
+ premium: { amount: string; currency: "ETH" };
+ total: { amount: string; currency: "ETH" };
+ }>,
+) {
+ const sum = (BigInt(ctx.value.baseCost.amount) + BigInt(ctx.value.premium.amount)).toString();
+ if (ctx.value.total.amount !== sum) {
+ ctx.issues.push({
+ code: "custom",
+ input: ctx.value,
+ message: `'total' must be equal to the sum of 'baseCost' and 'premium'`,
+ });
+ }
+}
+
const makeSerializedRegistrarActionPricingSchema = (
valueLabel: string = "Serialized Registrar Action Pricing",
) =>
z.union([
z.object({
baseCost: makeSerializedPriceEthSchema(`${valueLabel} Base Cost`),
premium: makeSerializedPriceEthSchema(`${valueLabel} Premium`),
total: makeSerializedPriceEthSchema(`${valueLabel} Total`),
- }),
+ }).check(invariant_serializedRegistrarActionPricingTotalIsSumOfBaseCostAndPremium),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ensnode-sdk/src/registrars/zod-schemas.ts` around lines 103 - 117,
The serialized registrar action pricing schema
(makeSerializedRegistrarActionPricingSchema) currently accepts objects where
baseCost, premium, and total are individually valid but not checked for
consistency; update makeSerializedRegistrarActionPricingSchema to add the same
validation parity as the domain pricing schema by adding a
refinement/superRefine that, for the non-null object variant produced from
makeSerializedPriceEthSchema, computes baseCost + premium and asserts it equals
total (and still allows the null-object variant), so inconsistent serialized
payloads are rejected early.
| const makeBaseSerializedRegistrarActionSchema = ( | ||
| valueLabel: string = "Serialized Base Registrar Action", | ||
| ) => | ||
| z | ||
| .object({ | ||
| id: EventIdSchema, | ||
| incrementalDuration: makeDurationSchema(`${valueLabel} Incremental Duration`), | ||
| registrant: makeLowercaseAddressSchema(`${valueLabel} Registrant`), | ||
| registrationLifecycle: makeRegistrationLifecycleSchema( | ||
| `${valueLabel} Registration Lifecycle`, | ||
| ), | ||
| pricing: makeSerializedRegistrarActionPricingSchema(`${valueLabel} Pricing`), | ||
| referral: makeRegistrarActionReferralSchema(`${valueLabel} Referral`), | ||
| block: makeBlockRefSchema(`${valueLabel} Block`), | ||
| transactionHash: makeTransactionHashSchema(`${valueLabel} Transaction Hash`), | ||
| eventIds: EventIdsSchema, | ||
| }) | ||
| .check(invariant_eventIdsInitialElementIsTheActionId); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract shared base action shape to prevent schema drift.
Line [191]-Line [206] and Line [231]-Line [243] duplicate the same fields with only pricing differing. Please factor a shared shape builder to keep invariants and field edits in one place.
As per coding guidelines: “Do not duplicate definitions across multiple locations; use type aliases to document invariants, with each invariant documented exactly once on its type alias.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ensnode-sdk/src/registrars/zod-schemas.ts` around lines 227 - 245,
Duplicate field definitions for serialized registrar actions should be extracted
into a single shared shape builder to avoid schema drift: create a helper (e.g.,
makeSerializedRegistrarActionBaseShape or makeBaseActionShape) that returns the
common z.object shape including id (EventIdSchema), incrementalDuration,
registrant (makeLowercaseAddressSchema), registrationLifecycle, referral
(makeRegistrarActionReferralSchema), block (makeBlockRefSchema), transactionHash
(makeTransactionHashSchema), and eventIds (EventIdsSchema) and apply
invariant_eventIdsInitialElementIsTheActionId to that shape; then update the
existing makeBaseSerializedRegistrarActionSchema and the other duplicated schema
to compose that base shape and only add the differing pricing field (using
makeSerializedRegistrarActionPricingSchema) so all common invariants are defined
once.
| transactionHash: makeTransactionHashSchema(`${valueLabel} Transaction Hash`), | ||
| eventIds: EventIdsSchema, | ||
| }) | ||
| .check(invariant_eventIdsInitialElementIsTheActionId); |
There was a problem hiding this comment.
not sure if I need to add this check here... I think it will be triggered during makeBaseRegistrarActionSchema, no? i'm still trying to understand how it all works
There was a problem hiding this comment.
@sevenzing Yes it hurts my head too. Suggest to check with @tk-o for his advice as I think he's the most familiar with this existing logic in our code.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/ensapi/src/lib/handlers/params.schema.ts (1)
70-100:⚠️ Potential issue | 🟠 MajorKeep accepting the legacy
namequery key at runtime.This is a runtime breaking change, not just an OpenAPI rename: because the outer query schema now only whitelists
nameRecord, legacy?name=trueis stripped beforeparams.selection.parse(...)runs. Existing callers can therefore lose the name record silently or getSelection cannot be empty.when that was their only selector. Keepnameas a deprecated runtime alias, or remap it tonameRecordbefore validation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensapi/src/lib/handlers/params.schema.ts` around lines 70 - 100, The selection schema currently only accepts nameRecord at runtime so legacy ?name=true is stripped; update the runtime handling to accept the deprecated "name" key and map it to the new field: add name: z.optional(boolstring) to the selection z.object and in the .transform for selection use the combined check (value.name ?? value.nameRecord) when building the ResolverRecordsSelection (i.e., set name: true if either value.name or value.nameRecord is truthy) so existing callers keep working while still supporting nameRecord.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/fair-ghosts-poke.md:
- Line 5: Update the changeset summary to improve grammar, capitalization, and
specificity: change the line to start with a capital letter, use present-tense
"Fixes" (e.g., "Fixes error handling in app.onError to return correct HTTP
status codes and resolves OpenAPI schema generation issues"), and include the
specific components affected (app.onError and OpenAPI schema generation) so the
changeset description is clear and actionable.
In `@docs/docs.ensnode.io/ensapi-openapi.json`:
- Around line 2423-2454: Add concrete example payloads for the 400 and 500
responses so the generated docs show error cases: update the OpenAPI response
objects that define the 400 and 500 schemas (the response entries with keys
"400" and "500") to include an "example" (or an "examples") value under
content.application/json (or directly under the schema) showing a realistic
payload — e.g. for 400 include { "message": "Invalid query", "details": { ... }
} and for 500 include { "responseCode": "error", "error": { "message": "Internal
server error", "details": { ... } } } — ensure the example fields align with the
existing schema properties ("message", "details", "responseCode", "error") so
regeneration will embed these examples in the docs.
---
Duplicate comments:
In `@apps/ensapi/src/lib/handlers/params.schema.ts`:
- Around line 70-100: The selection schema currently only accepts nameRecord at
runtime so legacy ?name=true is stripped; update the runtime handling to accept
the deprecated "name" key and map it to the new field: add name:
z.optional(boolstring) to the selection z.object and in the .transform for
selection use the combined check (value.name ?? value.nameRecord) when building
the ResolverRecordsSelection (i.e., set name: true if either value.name or
value.nameRecord is truthy) so existing callers keep working while still
supporting nameRecord.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 280cfc9e-b312-43c8-9b25-1f939cd1d4aa
📒 Files selected for processing (5)
.changeset/fair-ghosts-poke.mdapps/ensapi/src/lib/handlers/params.schema.tsapps/ensindexer/src/config/config.test.tsdocs/docs.ensnode.io/ensapi-openapi.jsonpackages/ensdb-sdk/src/client/zod-schemas/ensdb-config.ts
| "400": { | ||
| "description": "Invalid query", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "object", | ||
| "properties": { "message": { "type": "string" }, "details": {} }, | ||
| "required": ["message"] | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "500": { | ||
| "description": "Internal server error", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "object", | ||
| "properties": { | ||
| "responseCode": { "type": "string", "enum": ["error"] }, | ||
| "error": { | ||
| "type": "object", | ||
| "properties": { "message": { "type": "string" }, "details": {} }, | ||
| "required": ["message"] | ||
| } | ||
| }, | ||
| "required": ["responseCode", "error"], | ||
| "additionalProperties": false | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Add concrete 400/500 examples for the registrar-actions error responses.
These changed error media types now have schemas, but they still omit concrete example payloads, so the generated docs only show a fully rendered success case. Please add error examples at the route/schema source so regeneration emits them here.
Also applies to: 2969-3000
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/docs.ensnode.io/ensapi-openapi.json` around lines 2423 - 2454, Add
concrete example payloads for the 400 and 500 responses so the generated docs
show error cases: update the OpenAPI response objects that define the 400 and
500 schemas (the response entries with keys "400" and "500") to include an
"example" (or an "examples") value under content.application/json (or directly
under the schema) showing a realistic payload — e.g. for 400 include {
"message": "Invalid query", "details": { ... } } and for 500 include {
"responseCode": "error", "error": { "message": "Internal server error",
"details": { ... } } } — ensure the example fields align with the existing
schema properties ("message", "details", "responseCode", "error") so
regeneration will embed these examples in the docs.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
apps/ensapi/src/handlers/api/explore/registrar-actions-api.routes.ts
Outdated
Show resolved
Hide resolved
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@sevenzing Hey good to see the improvements here. Reviewed and shared feedback 👍
| @@ -75,7 +75,7 @@ app.get("/health", async (c) => { | |||
| // log hono errors to console | |||
| app.onError((error, ctx) => { | |||
There was a problem hiding this comment.
For this fallback catch-all error handler, what do you think about adding a prefix to the log messages / `errorResponse that would clearly identify for us that the error went through this fallback catch-all error handler.
Goal: Help us with debugging.
For example, maybe a little prefix such as "Unexpected server error: ..."?
Appreciate your advice.
| if (!url.startsWith("postgresql://") && !url.startsWith("postgres://")) { | ||
| export const EnsDbUrlSchema = z | ||
| .string({ | ||
| error: "ENV variable ENSDB_URL is required.", |
There was a problem hiding this comment.
There's a few updates in this file where we're making specific reference to the ENSDB_URL env variable or the ENSINDEXER_SCHEMA_NAME env variable.
It's really good that the error messages apps like ENSIndexer or ENSApi output in the case these env variables are incorrectly formatted make specific reference back to these env variable names. I really like that! It will help people who run these apps to debug incorrect env variable configurations.
However, we need to separate this logic into different layers of responsibility.
Please note how this code is living in the ensdb-sdk package. It should therefore not be tightly bound to the responsibilities that live within a particular app (ex: environment variable names or reading directly from environment variables). App-level responsibilities should live at the app-level, not at the package-level.
Therefore, suggest something like the following division of responsibilities:
- In the
ensdb-sdkpackage the logic here can output error information about what's invalid about an EnsDbUrl, or an EnsIndexerSchemaName, but it shouldn't make any hardcoded references to environment variable names. - We can then define a wrapper utility function that would extend the above parsing logic specifically for the context that it is related to an environment variable named X.
- It's fine to have this utility function defined in
ensdb-sdkso long as it (a) doesn't read directly from env vars itself; and (b) takes as input the env var name rather than hardcoding it.
- It's fine to have this utility function defined in
Appreciate your advice 👍
| schema: makeSerializedRegistrarActionsResponseSchema( | ||
| "Registrar Actions Response", | ||
| ).openapi({ | ||
| example: registrarActionsResponseOkExample, |
There was a problem hiding this comment.
It would be awesome if you could make it so that we could produce multiple examples in the OpenAPI spec. For example, both an Ok and and Error example.
It's important that we document examples for error cases too.
For some of our APIs, it would also be valuable if we could document various edge cases for success responses too.
What do you think? Please feel welcome to create a separate follow-up issue and PR for this goal.
|
|
||
| const makeSerializedPriceEthSchema = (valueLabel: string = "Serialized Price ETH") => | ||
| z.object({ | ||
| amount: z.string({ error: `${valueLabel} amount must be a string.` }).regex(/^\d+$/, { |
There was a problem hiding this comment.
We should have some other existing Zod schemas you could build upon for non-negative integers so that no regex is needed here.
Goal: Build upon the primitives we've already been creating.
| amount: z.string({ error: `${valueLabel} amount must be a string.` }).regex(/^\d+$/, { | ||
| error: `${valueLabel} amount must be a non-negative integer string.`, | ||
| }), | ||
| currency: z.literal("ETH", { |
There was a problem hiding this comment.
Hmm. I thought we would already have a Zod schema defined for this? Can you please review our existing Zod schemas such as:
makePriceSchemamakePriceEthSchemadeserializePriceEth
Perhaps there's existing code to reuse or refactor?
| */ | ||
| export const resolveRecordsResponseExample = { | ||
| records: { | ||
| name: "vitalik.eth", |
There was a problem hiding this comment.
Suggest we remove the name resolver record from being used in any of our forward-reslution examples. It will be confusing for 99% of devs and therefore create more problems than benefit to highlight in our example docs.
| }, | ||
| }, | ||
| accelerationRequested: false, | ||
| accelerationAttempted: false, |
There was a problem hiding this comment.
This terminology of accelerationAttempted is confusing. Can you please create a follow up issue and PR to update it?
The problem is that "attempted" is ambiguous. Ok.. maybe it was "attempted" but was it actually "completed"?
I assume that this field actually represents if acceleration was completed or not?
Assuming so, suggest to rename this field from accelerationAttempted to just accelerated as that is the more concise way of communicating the idea of "acceleration completed".
| "ensapi": patch | ||
| --- | ||
|
|
||
| use example requests for openapi doc |
There was a problem hiding this comment.
| use example requests for openapi doc | |
| add example responses to autogenerated openapi doc |
These are example responses, right?
.changeset/fair-ghosts-poke.md
Outdated
| "ensapi": patch | ||
| --- | ||
|
|
||
| fix handling error on latest server step and some minor bug fixes |
There was a problem hiding this comment.
Please update this changeset to make it more clear. I'm struggling to understand it.
| "description": "ENS name to resolve (e.g. 'vitalik.eth'). Must be normalized per ENSIP-15." | ||
| }, | ||
| "required": true, | ||
| "description": "ENS name to resolve (e.g. 'vitalik.eth'). Must be normalized per ENSIP-15.", |
There was a problem hiding this comment.
I note that the description is being duplicated both here and in the schema a few lines above.
Maybe this is totally fine and the right way to do things? I'm not sure. If it is, please ignore and mark this as resolved. Just flagging in case there's something for us to optimize here. Appreciate your advice. Thanks.
2 participants
Summary
app.onErrorto pass the actual error object instead of a hardcoded string, such thathttps://api.alpha.ensnode.io/api/resolve/records/vitalik.ethwont result in 500! (it will result in 400 but we will fix it soon too 👍 )name→nameRecordquery param in record selection of/api/resolve/records/to avoid collision with the name path paramexamples.tsfiles forname-tokens,registrar-actions, andresolution— typed example constants used in both routes and tests!resolution/registrar-actionsquery and path params; fixbeginTimestampandendTimestampmissing type:"integer"in OpenAPI output was missing for some reason, idk why...makeSerialized...Schema— wire-format schemas with string pricing amounts (separate from the domain bigint schemas) for use in OpenAPI route definitionsz.undefined()frommakeResponsePageContextSchemaWithNoRecords—ZodUndefinedhas no JSON Schema representation and caused UnknownZodTypeError during OpenAPI generationTesting
tested it with
Pre-Review Checklist (Blocking)