Conversation
Contributor
📝 WalkthroughWalkthroughAdds a TokenDetailService (interface, implementation, DI symbol), wires it into the API container, and exposes two new API routes: GET /details for a single token and POST /details for batch token lookups with JSON schema validation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client
participant API as API Route
participant DI as Inversify Container
participant Service as TokenDetailService
participant Repo as Erc20Repository
Client->>API: HTTP GET/POST /:chainId/tokens/.../details
API->>DI: resolve(tokenDetailServiceSymbol)
DI->>Service: provide TokenDetailServiceMain
API->>Service: getTokenDetails / getTokensDetails(chainId, address(es))
Service->>Repo: erc20Repository.get(chainId, address)
Repo-->>Service: Erc20 | null
Service-->>API: Erc20 | null (or list)
API-->>Client: 200 JSON or 404 {"message":"Token not found"}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
libs/services/src/TokenDetailService/TokenDetailService.ts (1)
43-47: ConsiderPromise.allSettledfor more resilient batch fetching.If a single
erc20Repository.getcall rejects (e.g., network error),Promise.allwill short-circuit and reject the entire batch. For a user-facing batch endpoint, partial success may be preferable—returning details for tokens that succeeded while excluding failures.♻️ Proposed alternative using Promise.allSettled
async getTokensDetails( chainId: string, tokenAddresses: string[] ): Promise<(Erc20 | null)[]> { const supportedChainId = toSupportedChainId(chainId); - return Promise.all( - tokenAddresses.map((address) => - this.erc20Repository.get(supportedChainId, address) - ) - ); + const results = await Promise.allSettled( + tokenAddresses.map((address) => + this.erc20Repository.get(supportedChainId, address) + ) + ); + return results.map((result) => + result.status === 'fulfilled' ? result.value : null + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/services/src/TokenDetailService/TokenDetailService.ts` around lines 43 - 47, The current batch fetch in TokenDetailService.ts uses Promise.all over tokenAddresses.map calling this.erc20Repository.get, which will reject the entire batch if any single fetch fails; change this to use Promise.allSettled on the mapped promises, then filter and return only the fulfilled results (extracting their values) while logging or handling rejected entries as needed so partial successes are returned from the method instead of a hard failure.apps/api/src/app/routes/__chainId/tokens/details.ts (1)
38-64: Consider extracting sharedtokenSchemaanderrorSchemato reduce duplication.The
tokenSchemahere is identical tosuccessSchemain__tokenAddress/details.ts, anderrorSchemais duplicated across both files. Extracting these to a shared location (e.g.,schemas.ts) would improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/routes/__chainId/tokens/details.ts` around lines 38 - 64, Duplicate JSON schemas (tokenSchema and errorSchema) are present; extract them to a shared module and import them where needed. Create a new exported file (e.g., schemas.ts) that exports tokenSchema and errorSchema (and reuse successSchema if present), update the current file to import tokenSchema and errorSchema instead of declaring them inline, and update the other file that declares successSchema/errorSchema to import from the same shared module so both routes reference the single source of truth.
🤖 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/api/src/app/routes/__chainId/tokens/__tokenAddress/details.ts`:
- Around line 87-101: The route handler's current flow lets toSupportedChainId
throw an unhandled exception for unknown chain IDs, causing a 500 instead of
returning a 400; update the async route handler (the function that calls
tokenDetailService.getTokenDetails) to catch the error from toSupportedChainId
(or wrap the chainId conversion call) and respond with reply.code(400).send({
message: 'Unsupported chainId' }) for that specific error, leaving other errors
to propagate or be handled as before; you can also move equivalent try/catch
into tokenDetailService.getTokenDetails if conversion happens there, but ensure
the handler returns 400 for unsupported chain ids.
In `@apps/api/src/app/routes/__chainId/tokens/details.ts`:
- Around line 114-130: The route handler currently uses request.params.chainId
directly and allowing toSupportedChainId to throw will produce a 500; wrap the
chain ID normalization in a try/catch (call toSupportedChainId(chainId)) at the
start of the async function and on error reply.code(400).send({ message:
'Unsupported chainId' }) (or include the error.message for more detail), then
use the validated/converted chainId when calling
tokenDetailService.getTokensDetails so the exception is handled and a proper 400
response is returned.
---
Nitpick comments:
In `@apps/api/src/app/routes/__chainId/tokens/details.ts`:
- Around line 38-64: Duplicate JSON schemas (tokenSchema and errorSchema) are
present; extract them to a shared module and import them where needed. Create a
new exported file (e.g., schemas.ts) that exports tokenSchema and errorSchema
(and reuse successSchema if present), update the current file to import
tokenSchema and errorSchema instead of declaring them inline, and update the
other file that declares successSchema/errorSchema to import from the same
shared module so both routes reference the single source of truth.
In `@libs/services/src/TokenDetailService/TokenDetailService.ts`:
- Around line 43-47: The current batch fetch in TokenDetailService.ts uses
Promise.all over tokenAddresses.map calling this.erc20Repository.get, which will
reject the entire batch if any single fetch fails; change this to use
Promise.allSettled on the mapped promises, then filter and return only the
fulfilled results (extracting their values) while logging or handling rejected
entries as needed so partial successes are returned from the method instead of a
hard failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb984f19-897e-4a8e-afb0-d7eb472bc584
⛔ Files ignored due to path filters (1)
libs/repositories/src/gen/cow/cow-api-types.tsis excluded by!**/gen/**
📒 Files selected for processing (5)
apps/api/src/app/inversify.config.tsapps/api/src/app/routes/__chainId/tokens/__tokenAddress/details.tsapps/api/src/app/routes/__chainId/tokens/details.tslibs/services/src/TokenDetailService/TokenDetailService.tslibs/services/src/index.ts
Switch from ChainIdOrSlugSchema to SupportedChainIdSchema so Fastify rejects unsupported chains with a 400 validation error instead of letting them through to the service where they cause a 500.
Contributor
Author
|
@alfetopito @shoom3301 I need this endpoints for a new service I'm building. I will merge to staging to be able to use it. I did a detailed test plan and test it. Let me know if I need to address anything. Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a couple of endpoints to return token details (symbol, name, decimals, etc)
GET /:chainId/tokens/:tokenAddress/detailsendpoint to get details (name, symbol, decimals) for a single tokenPOST /:chainId/tokens/detailsendpoint to get details for multiple tokens (up to 100) in a single requestTokenDetailServicewrapping the existingErc20RepositoryTest plan
Single token - WETH on Mainnet
Single token - USDC on Mainnet
Single token - non-existent address (expect 404)
curl -s -w "\nHTTP status: %{http_code}\n" http://localhost:3001/1/tokens/0x0000000000000000000000000000000000000001/detailsSingle token - unsupported chain (expect 400)
curl -s -w "\nHTTP status: %{http_code}\n" http://localhost:3001/999/tokens/0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2/detailsSingle token - invalid address format (expect 400)
curl -s -w "\nHTTP status: %{http_code}\n" http://localhost:3001/1/tokens/not-an-address/detailsMultiple tokens on Mainnet
Multiple tokens - unsupported chain (expect 400)
Multiple tokens - empty array (expect 400)
Multiple tokens - invalid address in array (expect 400)