refactor(ensapi): apply operation result pattern #1545
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
@greptile review |
There was a problem hiding this comment.
Pull request overview
This PR applies the operation result pattern from ENSNode SDK to standardize response handling across ENSApi endpoints. The changes introduce new result types and builder functions, and refactor error handling to use a consistent pattern.
Changes:
- Introduced new result pattern infrastructure in the SDK with
buildResultOk,buildResultOkTimestamped, andbuildResultServiceUnavailablebuilders - Added
ServiceUnavailableresult code and updated result type definitions - Refactored ENSApi endpoints (
/amirealtime,/api/registrar-actions) to use the result pattern with newresultIntoHttpResponseutility - Removed legacy
errorResponsefunction in favor of standardized result handling
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ensnode-sdk/src/shared/result/result-common.ts | Added buildResultOk, buildResultOkTimestamped, buildResultServiceUnavailable builders and new result type definitions |
| packages/ensnode-sdk/src/shared/result/result-code.ts | Added ServiceUnavailable result code |
| packages/ensnode-sdk/src/shared/result/result-base.ts | Added AbstractResultOkTimestamped type with indexing cursor support |
| packages/ensnode-sdk/src/api/registrar-actions/serialize.ts | Added serializeNamedRegistrarActions helper function |
| packages/ensnode-sdk/src/api/registrar-actions/response.ts | Added RegistrarActionsResultOkData and RegistrarActionsResult types |
| apps/ensapi/src/middleware/registrar-actions.middleware.ts | Updated to use result pattern for error handling |
| apps/ensapi/src/lib/result/result-into-http-response.ts | New utility to convert operation results to HTTP responses |
| apps/ensapi/src/lib/result/result-into-http-response.test.ts | Comprehensive test coverage for result-to-HTTP conversion |
| apps/ensapi/src/lib/handlers/validate.ts | Updated validation error handling to use result pattern |
| apps/ensapi/src/lib/handlers/error-response.ts | Removed legacy error response handler |
| apps/ensapi/src/index.ts | Updated error handlers and notFound handler to use result pattern |
| apps/ensapi/src/handlers/registrar-actions-api.ts | Refactored to use result pattern for responses |
| apps/ensapi/src/handlers/amirealtime-api.ts | Refactored to use result pattern for responses |
| apps/ensapi/src/handlers/amirealtime-api.test.ts | Updated tests to verify new result pattern behavior |
Comments suppressed due to low confidence (1)
apps/ensapi/src/lib/handlers/validate.ts:14
- The documentation comment still references the old
errorResponsefunction which has been replaced with the result pattern. Update the comment to reflect that it now usesbuildResultInvalidRequestandresultIntoHttpResponsefor consistent error formatting.
/**
* Creates a Hono validation middleware with custom error formatting.
*
* Wraps the Hono validator with custom error handling that uses the
* errorResponse function for consistent error formatting across the API.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile OverviewGreptile SummaryThis PR successfully refactors ENSApi to adopt a standardized result pattern for HTTP responses, replacing ad-hoc error handling with a consistent data model across all endpoints. Key Changes
Architecture BenefitsThe result pattern provides several improvements:
All previous review comments about field name mismatches have been addressed - the codebase now consistently uses Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant HonoRouter
participant ValidationMiddleware
participant RegistrarActionsMiddleware
participant RouteHandler
participant resultIntoHttpResponse
participant ENSIndexer
Client->>HonoRouter: GET /api/registrar-actions/:parentNode
HonoRouter->>ValidationMiddleware: validate(param, schema)
alt validation fails
ValidationMiddleware->>ValidationMiddleware: buildResultInvalidRequest(errorMessage)
ValidationMiddleware->>resultIntoHttpResponse: resultIntoHttpResponse(c, result)
resultIntoHttpResponse->>resultIntoHttpResponse: resultCodeToHttpStatusCode(400)
resultIntoHttpResponse->>Client: Response 400 (InvalidRequest)
end
ValidationMiddleware->>RegistrarActionsMiddleware: check prerequisites
alt insufficient indexing progress
RegistrarActionsMiddleware->>RegistrarActionsMiddleware: buildResultInsufficientIndexingProgress(...)
RegistrarActionsMiddleware->>resultIntoHttpResponse: resultIntoHttpResponse(c, result)
resultIntoHttpResponse->>Client: Response 503 (InsufficientIndexingProgress)
else service unavailable
RegistrarActionsMiddleware->>RegistrarActionsMiddleware: buildResultServiceUnavailable(...)
RegistrarActionsMiddleware->>resultIntoHttpResponse: resultIntoHttpResponse(c, result)
resultIntoHttpResponse->>Client: Response 503 (ServiceUnavailable)
end
RegistrarActionsMiddleware->>RouteHandler: next()
RouteHandler->>ENSIndexer: findRegistrarActions(filters, orderBy, limit, offset)
ENSIndexer-->>RouteHandler: {registrarActions, totalRecords}
RouteHandler->>RouteHandler: buildPageContext(page, recordsPerPage, totalRecords)
RouteHandler->>RouteHandler: buildRegistrarActionsResultOk(data, minIndexingCursor)
RouteHandler->>RouteHandler: serializeRegistrarActionsResultOk(result)
RouteHandler->>resultIntoHttpResponse: resultIntoHttpResponse(c, serializedResult)
resultIntoHttpResponse->>resultIntoHttpResponse: resultCodeToHttpStatusCode(200)
resultIntoHttpResponse->>Client: Response 200 (Ok with data and minIndexingCursor)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
tk-o
force-pushed
the
ensnode-result-type/registrar-actions-api
branch
from
745bf8b to
902d556
Compare
|
@greptile re-review |
tk-o
force-pushed
the
ensnode-result-type/registrar-actions-api
branch
from
e626f17 to
412e722
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ive HTTP endpoint docs
tk-o
force-pushed
the
ensnode-result-type/registrar-actions-api
branch
from
bd0c21c to
50b5e18
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 37 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@greptile review |
tk-o
left a comment
tk-o
left a comment
There was a problem hiding this comment.
Self-review completed
| const priceAmountSchemaSerializable = z.string(); | ||
| const makePriceAmountSchemaNative = (valueLabel: string = "Price Amount") => | ||
| z.preprocess( | ||
| (v) => (typeof v === "string" ? BigInt(v) : v), | ||
| z | ||
| .bigint({ | ||
| error: `${valueLabel} must represent a bigint.`, | ||
| }) | ||
| .nonnegative({ | ||
| error: `${valueLabel} must not be negative.`, | ||
| }), | ||
| ); | ||
|
|
||
| export const makePriceCurrencySchema = ( | ||
| export function makePriceAmountSchema<const SerializableType extends boolean>( | ||
| valueLabel: string, | ||
| serializable: SerializableType, | ||
| ): SerializableType extends true | ||
| ? typeof priceAmountSchemaSerializable | ||
| : ReturnType<typeof makePriceAmountSchemaNative>; | ||
| export function makePriceAmountSchema( | ||
| valueLabel: string = "Price Amount Schema", | ||
| serializable: true | false = false, | ||
| ): typeof priceAmountSchemaSerializable | ReturnType<typeof makePriceAmountSchemaNative> { | ||
| if (serializable) { | ||
| return priceAmountSchemaSerializable; | ||
| } else { | ||
| return makePriceAmountSchemaNative(valueLabel); | ||
| } | ||
| } |
There was a problem hiding this comment.
Applied a pattern of making Zod schema "serializable" for the purpose of generating OpenAPI spec. Without that conditional approach, the OpenAPI spec generation process would fail due to unsupported z.bigint() schema.
| makeRegistrarActionRegistrationSchema(`${valueLabel} Registration`, serializable), | ||
| makeRegistrarActionRenewalSchema(`${valueLabel} Renewal`, serializable), |
There was a problem hiding this comment.
Applied a pattern of making Zod schema "serializable" for the purpose of generating OpenAPI spec. All updates in this file follow this one.
| z.union([ | ||
| makeResponsePageContextSchemaWithNoRecords(valueLabel), | ||
| makeResponsePageContextSchemaWithRecords(valueLabel), | ||
| makeResponsePageContextSchemaWithNoRecords(valueLabel), |
There was a problem hiding this comment.
Changes in this file were required to make the response page context schema "serializable" for the purpose of generating OpenAPI spec.
|
|
||
| /** | ||
| * The start index of the records on the page (0-indexed) | ||
| */ | ||
| startIndex: undefined; | ||
|
|
||
| /** | ||
| * The end index of the records on the page (0-indexed) | ||
| */ | ||
| endIndex: undefined; |
There was a problem hiding this comment.
Changes in this file were required to make the response page context schema "serializable" for the purpose of generating OpenAPI spec.
| startIndex: undefined, | ||
| endIndex: undefined, |
There was a problem hiding this comment.
Changes in this file were required to make the response page context schema "serializable" for the purpose of generating OpenAPI spec.
|
|
||
| // fetching is temporarily not possible due to indexing status being not advanced enough | ||
| if (!hasIndexingStatusSupport(omnichainSnapshot.omnichainStatus)) { | ||
| if (!hasIndexingStatusSupport(configTypeId, omnichainSnapshot.omnichainStatus)) { |
There was a problem hiding this comment.
Checking indexing status support now requires providing the chains configuration type ID.
| return { | ||
| fetchStatus: StatefulFetchStatusIds.NotReady, | ||
| supportedIndexingStatusIds, | ||
| supportedIndexingStatusId: getSupportedIndexingStatus(configTypeId), |
There was a problem hiding this comment.
There can be just one supported indexing status at a time.
| supportedIndexingStatusId: | ||
| | typeof OmnichainIndexingStatusIds.Completed | ||
| | typeof OmnichainIndexingStatusIds.Following; |
There was a problem hiding this comment.
There can be just one supported indexing status at a time.
| The Registrar Actions API will be available once the omnichain indexing status becomes{" "} | ||
| <Badge variant="secondary"> | ||
| {formatOmnichainIndexingStatus(registrarActions.supportedIndexingStatusId)} | ||
| </Badge> | ||
| . |
There was a problem hiding this comment.
There can be just one supported indexing status at a time.
| { | ||
| fetchStatus: StatefulFetchStatusIds.NotReady, | ||
| supportedIndexingStatusIds: registrarActionsPrerequisites.supportedIndexingStatusIds, | ||
| supportedIndexingStatusId: OmnichainIndexingStatusIds.Following, |
There was a problem hiding this comment.
There can be just one supported indexing status at a time.
|
@greptile review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Lite PR
Summary
resultIntoHttpResponsefunction, that produces a HTTPResponseobject based on the provided operation result.EnsNodeResponse<T>JSON response schema #1305, Streamline response codes across API modules #1355onError,notFound)GET /healthendpointHealthServerResultresult typeGET /amirealtimeendpointAmIRealtimeServerResultresult typeRegistrarActionsServerResultresult typeminIndexingCursor(as per MoveaccurateAsOffield into generic response data model / middleware #1512). We useAbstractResultOkTimestampedtype to achieve outcome. SeeRegistrarActionsResultOkresult type.RESULT_CODE_SERVER_ERROR_CODESwithResultCodes.ServiceUnavailableResultCodes.InsufficientIndexingProgressWhy
minIndexingCursorfield in their response data model to indicate to which point in time the response data is guaranteed to be up to.Testing
/amirealtimeand/api/registrar-actionsendpoints locally, simulating conditions where both, ok and error results were returned from ENSApi.Please find most important HTTP API test cases below.
HTTP API test cases
"Am I Realtime?" API
AmIRealtimeResultOk
Request
Response
{ "resultCode": "ok", "data": { "maxWorstCaseDistance": 22, "slowestChainIndexingCursor": 1769091731, "worstCaseDistance": 9 } }ResultInsufficientIndexingProgress
Request
Response
{ "resultCode": "insufficient-indexing-progress", "suggestRetry": true, "errorMessage": "Indexing Status 'worstCaseDistance' must be below or equal to the requested 'maxWorstCaseDistance'; worstCaseDistance = 14; maxWorstCaseDistance = 10", "data": { "indexingStatus": "omnichain-following", "slowestChainIndexingCursor": 1769091851, "earliestChainIndexingCursor": 1489165544, "progressSufficientFrom": { "indexingStatus": "omnichain-following", "chainIndexingCursor": 1769091855 } } }Registrar Actions API
RegistrarActionsResultOk
Request
Response
{ "resultCode": "ok", "data": { "registrarActions": [ { "action": { "id": "176909197100000000000000010000000024290909000000000000004950000000000000273", "type": "registration", "incrementalDuration": 20736000, "registrant": "0xeb087f37d87bf6d855717b056519c18e6761e68c", "registrationLifecycle": { "subregistry": { "subregistryId": { "chainId": 1, "address": "0x57f1887a8bf19b14fc0df6fd9b2acc9af147ea85" }, "node": "0x93cdeb708b7545dc668eb9280176169d1c33cfd8ed6f04690a0bcc88a93fc4ae" }, "node": "0xc2ab751f24da3537432be8a0b51086bdb2e55945d89bd3ff488d5ca2f450bf00", "expiresAt": 1789827971 }, "pricing": { "baseCost": { "currency": "ETH", "amount": "1104307319092065" }, "premium": { "currency": "ETH", "amount": "0" }, "total": { "currency": "ETH", "amount": "1104307319092065" } }, "referral": { "encodedReferrer": "0x0000000000000000000000007e491cde0fbf08e51f54c4fb6b9e24afbd18966d", "decodedReferrer": "0x7e491cde0fbf08e51f54c4fb6b9e24afbd18966d" }, "block": { "number": 24290909, "timestamp": 1769091971 }, "transactionHash": "0xc0a1f83ad441f430db99b87bd1d48d9eede5afa12e1723f8edc21850e86e2341", "eventIds": [ "176909197100000000000000010000000024290909000000000000004950000000000000273", "176909197100000000000000010000000024290909000000000000004950000000000000277" ] }, "name": "qualifiedinvestor.eth" }, { "action": { "id": "176909081900000000000000010000000024290813000000000000005050000000000000170", "type": "registration", "incrementalDuration": 31536000, "registrant": "0x718f6a369239c6963fd7807c62face162f8a31bf", "registrationLifecycle": { "subregistry": { "subregistryId": { "chainId": 1, "address": "0x57f1887a8bf19b14fc0df6fd9b2acc9af147ea85" }, "node": "0x93cdeb708b7545dc668eb9280176169d1c33cfd8ed6f04690a0bcc88a93fc4ae" }, "node": "0x5bbab16b388d2b32deb7d0fafefdd3997ba775dbd2c195b760d165c8bc5be8ff", "expiresAt": 1800626819 }, "pricing": { "baseCost": { "currency": "ETH", "amount": "1688390191103095" }, "premium": { "currency": "ETH", "amount": "0" }, "total": { "currency": "ETH", "amount": "1688390191103095" } }, "referral": { "encodedReferrer": "0x0000000000000000000000007e491cde0fbf08e51f54c4fb6b9e24afbd18966d", "decodedReferrer": "0x7e491cde0fbf08e51f54c4fb6b9e24afbd18966d" }, "block": { "number": 24290813, "timestamp": 1769090819 }, "transactionHash": "0xccd876afe2b4391cc85df58b0f784cf6404359c8ce170fd7608e23b971b058d4", "eventIds": [ "176909081900000000000000010000000024290813000000000000005050000000000000170", "176909081900000000000000010000000024290813000000000000005050000000000000173" ] }, "name": "techfund.eth" }, { "action": { "id": "176909075900000000000000010000000024290808000000000000006650000000000000217", "type": "renewal", "incrementalDuration": 5184000, "registrant": "0x8b24b1686832757e2f6d640e11e88e7f0064594a", "registrationLifecycle": { "subregistry": { "subregistryId": { "chainId": 1, "address": "0x57f1887a8bf19b14fc0df6fd9b2acc9af147ea85" }, "node": "0x93cdeb708b7545dc668eb9280176169d1c33cfd8ed6f04690a0bcc88a93fc4ae" }, "node": "0x45477e804caf0ed4048bdb5602eb9e4f85ac4c341745ab301ec5be0ca5e3aff5", "expiresAt": 1770530400 }, "pricing": { "baseCost": { "currency": "ETH", "amount": "8881394977846876" }, "premium": { "currency": "ETH", "amount": "0" }, "total": { "currency": "ETH", "amount": "8881394977846876" } }, "referral": { "encodedReferrer": "0x0000000000000000000000007e491cde0fbf08e51f54c4fb6b9e24afbd18966d", "decodedReferrer": "0x7e491cde0fbf08e51f54c4fb6b9e24afbd18966d" }, "block": { "number": 24290808, "timestamp": 1769090759 }, "transactionHash": "0xa050c6977805dccd03831ecd8ec9d002c2fafe15b662cf74d5d23783bf37aa9a", "eventIds": [ "176909075900000000000000010000000024290808000000000000006650000000000000217", "176909075900000000000000010000000024290808000000000000006650000000000000218", "176909075900000000000000010000000024290808000000000000006650000000000000219" ] }, "name": "5585.eth" } ], "pageContext": { "page": 1, "recordsPerPage": 3, "totalRecords": 19714, "totalPages": 6572, "hasNext": true, "hasPrev": false, "startIndex": 0, "endIndex": 2 } }, "minIndexingCursor": 1769092355 }ResultInsufficientIndexingProgress
Request
Response
{ "resultCode": "insufficient-indexing-progress", "suggestRetry": true, "errorMessage": "Registrar Actions API is not available. The cached omnichain indexing status of the connected ENSIndexer has insufficient progress.", "data": { "indexingStatus": "omnichain-backfill", "slowestChainIndexingCursor": 1702676652, "earliestChainIndexingCursor": 1686901632, "progressSufficientFrom": { "indexingStatus": "omnichain-following", "chainIndexingCursor": 1769092140 } } }ResultServiceUnavailable
Request
Response
{ "resultCode": "service-unavailable", "errorMessage": "Registrar Actions API is not available. Connected ENSIndexer configuration does not have all required plugins active. Current plugins: \"subgraph\". Required plugins: \"subgraph, basenames, lineanames, registrars\".", "suggestRetry": true }Notes for Reviewer (Optional)
z.bigint()schema, it needs to be replaced with thez.string()"serialized" couterpart. That's why we use<const SerializableType extends boolean>parameter type to keep using single Zod schema definition, but with conditional serialization.Pre-Review Checklist (Blocking)
Resulttype #1539 before PR 1539 is ready for review.Resulttype #1539PR Creation Tips
Related to #1305, #1355, #1368, #1512.