refactor: adjusts authz and provider services to work when blockchain is down#1977
refactor: adjusts authz and provider services to work when blockchain is down#1977
Conversation
WalkthroughAuthzHttpService was refactored to accept an injected HttpClient and expose isReady; DI registrations updated to construct it from the shared chain API HttpClient. ProviderService now looks up Provider by owner from the database instead of using ProviderHttpService. Web queries and tests now gate on authzHttpService.isReady. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant DI as DI Container
participant HC as HttpClient (CHAIN_API_HTTP_CLIENT)
participant Authz as AuthzHttpService
App->>DI: initialize()
DI->>HC: createHttpClient(...)
DI->>Authz: new AuthzHttpService(HC)
App->>Authz: isReady?
Authz-->>App: !!HC.defaults.baseURL
Note over App,Authz: Queries enabled only when isReady is true
sequenceDiagram
autonumber
participant Caller
participant ProvSvc as ProviderService
participant DB as Database
participant ProviderAPI as Provider API
Caller->>ProvSvc: sendManifest(providerAddress, dseq, manifest, opts)
ProvSvc->>DB: findOne({ owner: providerAddress })
alt Provider found
ProvSvc->>ProviderAPI: POST /manifest (hostUri from DB, identity.owner=providerAddress)
ProviderAPI-->>ProvSvc: 2xx
ProvSvc-->>Caller: success
else Provider not found
ProvSvc-->>Caller: throw "Provider not found: providerAddress"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (4)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
**/*.{js,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
🧬 Code graph analysis (2)apps/api/src/core/providers/http-sdk.provider.ts (2)
packages/http-sdk/src/authz/authz-http.service.ts (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (3)
Comment |
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (25.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1977 +/- ##
==========================================
- Coverage 45.44% 45.43% -0.02%
==========================================
Files 1003 1003
Lines 28446 28443 -3
Branches 7465 7449 -16
==========================================
- Hits 12928 12922 -6
+ Misses 15242 15155 -87
- Partials 276 366 +90
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/deploy-web/src/queries/useGrantsQuery.spec.tsx (2)
146-155: Removeas any; use deep mocks for AxiosAvoid
anyand shape Axios via jest-mock-extended’s deep mocks. As per coding guidelines.- const chainApiHttpClient = mock<AxiosInstance>({ - defaults: { baseURL: "https://api.akash.network" }, - get: jest.fn().mockResolvedValue({ - data: { - allowances: mockData, - pagination: { next_key: null, total: mockData.length } - } - }) - } as any); + // at top: import { mock, mockDeep, DeepMocked } from "jest-mock-extended"; + const chainApiHttpClient: DeepMocked<AxiosInstance> = mockDeep<AxiosInstance>(); + chainApiHttpClient.defaults.baseURL = "https://api.akash.network"; + chainApiHttpClient.get.mockResolvedValue({ + data: { allowances: mockData, pagination: { next_key: null, total: mockData.length } } + } as never);
172-181: Same here — dropas anyfor the second case- const chainApiHttpClient = mock<AxiosInstance>({ - defaults: { baseURL: "https://api.akash.network" }, - get: jest.fn().mockResolvedValue({ - data: { - allowances: [], - pagination: { next_key: null, total: 0 } - } - }) - } as any); + const chainApiHttpClient: DeepMocked<AxiosInstance> = mockDeep<AxiosInstance>(); + chainApiHttpClient.defaults.baseURL = "https://api.akash.network"; + chainApiHttpClient.get.mockResolvedValue({ + data: { allowances: [], pagination: { next_key: null, total: 0 } } + } as never);
🧹 Nitpick comments (4)
apps/deploy-web/src/queries/useGrantsQuery.ts (2)
20-21: Gating via isReady looks right; avoid mutating incoming optionsPrefer not mutating the caller’s options object. Compute enabled and pass it into useQuery so our gating logic always wins.
@@ export function useGranterGrants(... - options.enabled = options.enabled !== false && !!address && authzHttpService.isReady; + const enabled = options.enabled !== false && !!address && authzHttpService.isReady; return useQuery({ queryKey: QueryKeys.getGranterGrants(address, page, offset), queryFn: () => authzHttpService.getPaginatedDepositDeploymentGrants({ granter: address, limit, offset }), - ...options + ...options, + enabled }); @@ export function useGranteeGrants(... - options.enabled = options.enabled !== false && !!address && authzHttpService.isReady; + const enabled = options.enabled !== false && !!address && authzHttpService.isReady; return useQuery({ - queryKey: QueryKeys.getGranteeGrants(address || "UNDEFINED"), + queryKey: address ? QueryKeys.getGranteeGrants(address) : [], queryFn: () => authzHttpService.getAllDepositDeploymentGrants({ grantee: address, limit: 1000 }), - ...options + ...options, + enabled }); @@ export function useAllowancesIssued(... - options.enabled = options.enabled !== false && !!address && authzHttpService.isReady; + const enabled = options.enabled !== false && !!address && authzHttpService.isReady; return useQuery({ queryKey: QueryKeys.getAllowancesIssued(address, page, offset), queryFn: () => authzHttpService.getPaginatedFeeAllowancesForGranter(address, limit, offset), - ...options + ...options, + enabled });Also applies to: 32-33, 50-51
35-36: Unify queryKey pattern for empty addressYou already use the “empty key disables” pattern elsewhere. Consider replacing "UNDEFINED" with [] for consistency, as shown in the diff above.
packages/http-sdk/src/authz/authz-http.service.ts (2)
60-61: Remove unused FEE_ALLOWANCE_TYPEIt’s declared but not referenced.
- private readonly FEE_ALLOWANCE_TYPE: FeeAllowance["allowance"]["@type"] = "/cosmos.feegrant.v1beta1.BasicAllowance";
153-161: Tighten type of nextPageKeyUse a concrete type to self-document expectations from the API.
- ): Promise<void> { - let nextPageKey: unknown; + ): Promise<void> { + let nextPageKey: string | null | undefined;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/api/src/core/providers/http-sdk.provider.ts(1 hunks)apps/api/src/provider/services/provider/provider.service.ts(2 hunks)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx(1 hunks)apps/deploy-web/src/queries/useGrantsQuery.spec.tsx(6 hunks)apps/deploy-web/src/queries/useGrantsQuery.ts(3 hunks)packages/http-sdk/src/authz/authz-http.service.ts(8 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/deploy-web/src/queries/useGrantsQuery.spec.tsx
apps/{deploy-web,provider-console}/**/*.spec.tsx
📄 CodeRabbit inference engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations in.spec.tsxfiles
Files:
apps/deploy-web/src/queries/useGrantsQuery.spec.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/deploy-web/src/queries/useGrantsQuery.spec.tsxapps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsxpackages/http-sdk/src/authz/authz-http.service.tsapps/api/src/provider/services/provider/provider.service.tsapps/api/src/core/providers/http-sdk.provider.tsapps/deploy-web/src/queries/useGrantsQuery.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/deploy-web/src/queries/useGrantsQuery.spec.tsxapps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsxpackages/http-sdk/src/authz/authz-http.service.tsapps/api/src/provider/services/provider/provider.service.tsapps/api/src/core/providers/http-sdk.provider.tsapps/deploy-web/src/queries/useGrantsQuery.ts
🧠 Learnings (1)
📚 Learning: 2025-07-21T08:24:27.953Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-07-21T08:24:27.953Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Applied to files:
apps/deploy-web/src/queries/useGrantsQuery.spec.tsx
🧬 Code graph analysis (4)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
packages/http-sdk/src/authz/authz-http.service.ts (1)
AuthzHttpService(56-219)
packages/http-sdk/src/authz/authz-http.service.ts (1)
packages/http-sdk/src/http/http.service.ts (2)
extractData(12-14)extractData(17-19)
apps/api/src/provider/services/provider/provider.service.ts (3)
apps/indexer/drizzle/schema.ts (1)
provider(205-237)apps/api/src/provider/services/provider/provider-proxy.service.ts (1)
ProviderIdentity(7-10)apps/api/src/deployment/http-schemas/lease.schema.ts (1)
LeaseStatusResponse(53-53)
apps/api/src/core/providers/http-sdk.provider.ts (2)
packages/http-sdk/src/utils/httpClient.ts (1)
HttpClient(23-23)packages/http-sdk/src/authz/authz-http.service.ts (1)
AuthzHttpService(56-219)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: Validate local packages
🔇 Additional comments (7)
apps/api/src/core/providers/http-sdk.provider.ts (2)
27-29: Authz removed from axios-based SERVICES — LGTMThis aligns with the refactor to a constructor that accepts HttpClient.
30-32: Add fallbackable HTTP client for NON_AXIOS_SERVICES
AuthzHttpService is registered with the plain CHAIN_API_HTTP_CLIENT in apps/api/src/core/providers/http-sdk.provider.ts; no fallback or indexer-based client was found—inject the Web app’s fallbackable HTTP client or a dedicated indexer client for Authz paths to handle blockchain downtime.apps/deploy-web/src/queries/useGrantsQuery.spec.tsx (1)
31-33: Updated gating to isReady — LGTMMocks now set isReady: true; matches the production readiness check.
Also applies to: 49-51, 72-74, 91-93, 111-113, 130-132
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
34-36: AuthzHttpService now built from shared chainApiHttpClient — LGTMMatches the HttpClient-based service.
The fallback currently points to externalApiHttpClient with a TODO to switch to the indexer client. Confirm indexer has the required cosmos/authz endpoints, or adjust the fallback target.
packages/http-sdk/src/authz/authz-http.service.ts (3)
56-66: Constructor DI + isReady getter — LGTMClean separation from HttpService and clear readiness semantics.
68-71: Endpoints and pagination parsing — LGTMEndpoints, params, and count_total parsing look correct; filters apply only valid/recognized grants.
Also applies to: 98-108, 111-121, 123-133, 161-171, 189-199
1-5: No.defaults.baseURLreferences onauthzHttpService; drop repo-wide follow-up.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/api/src/provider/services/provider/provider.service.ts (1)
65-74: Preserve HTTP status on provider proxy errors; avoidanyin catchGeneric
throw new Error(...)collapses provider errors into 500s and the catch usesany. Narrow errors and map Axios failures to appropriate HTTP codes.As per coding guidelines.
- } catch (err: any) { - if (err.message?.includes("no lease for deployment") && i < this.MANIFEST_SEND_MAX_RETRIES) { + } catch (err: unknown) { + const axiosErr = AxiosError.isAxiosError(err) ? err : undefined; + const msg = axiosErr?.message ?? (err instanceof Error ? err.message : undefined); + if (msg?.includes("no lease for deployment") && i < this.MANIFEST_SEND_MAX_RETRIES) { await delay(this.MANIFEST_SEND_RETRY_DELAY); continue; } - const providerError = err instanceof AxiosError && err.response?.data; - assert(!providerError?.toLowerCase()?.includes("invalid manifest"), 400, err?.response?.data); - - throw new Error(providerError || err); + const providerError = typeof axiosErr?.response?.data === "string" ? axiosErr.response.data : undefined; + assert(!providerError?.toLowerCase()?.includes("invalid manifest"), 400, providerError ?? msg); + if (axiosErr) { + const status = + axiosErr.response?.status ?? + (axiosErr.code === "ECONNABORTED" ? 504 : 502); + assert(false, status, providerError ?? axiosErr.message); + } + assert(false, 500, msg ?? "Internal Server Error"); }packages/http-sdk/src/authz/authz-http.service.ts (1)
157-170: Type safety + omit null/undefined “pagination.key”.Using unknown in params risks serialization of null/undefined; type it and include conditionally.
- let nextPageKey: unknown; + let nextPageKey: string | undefined; @@ - const response = extractData( - await this.httpClient.get<DepositDeploymentGrantResponse>(`cosmos/authz/v1beta1/grants/${side}/${address}`, { - params: { "pagination.key": nextPageKey, "pagination.limit": options.limit } - }) - ); + const response = extractData( + await this.httpClient.get<DepositDeploymentGrantResponse>(`cosmos/authz/v1beta1/grants/${side}/${address}`, { + params: { + ...(nextPageKey ? { "pagination.key": nextPageKey } : {}), + "pagination.limit": options.limit + } + }) + ); @@ - } while (nextPageKey); + } while (nextPageKey);
🧹 Nitpick comments (8)
apps/api/src/provider/services/provider/provider.service.ts (3)
39-41: Also filter out deleted providersBoth lookups should exclude logically deleted providers to avoid using stale
hostUri.Apply this diff to both queries:
-const provider = await Provider.findOne({ where: { owner: providerAddress } }); +const provider = await Provider.findOne({ where: { owner: providerAddress, deletedHeight: null } });-const provider = await Provider.findOne({ - where: { - owner: providerAddress - } -}); +const provider = await Provider.findOne({ + where: { owner: providerAddress, deletedHeight: null } +});Also applies to: 85-91
51-76: Minor: remove truthy guard on result
fetchProviderUrlreturning a falsy payload (e.g., empty object/string) would skip return. Prefer an unconditional return on success path.- if (result) return result; + return result;
36-49: DRY: factor provider lookup into a helper
findOne+ 404 repeat across methods. Extract to a private helper for consistency and easier future changes (e.g., additional filters/joins).Example:
private async getActiveProviderOr404(owner: string) { const p = await Provider.findOne({ where: { owner, deletedHeight: null } }); assert(p, 404, `Provider ${owner} not found`); return p!; }Then use it in both methods.
Also applies to: 78-105
packages/http-sdk/src/authz/authz-http.service.ts (5)
64-66: Harden readiness check to avoid runtime if defaults is absent.Guard against clients without Axios-like defaults.
- get isReady(): boolean { - return !!this.httpClient.defaults.baseURL; - } + get isReady(): boolean { + return Boolean(this.httpClient.defaults?.baseURL); + }
100-108: Prefer status‑code check over brittle message matching for “not found”.Indexer/chain nodes may vary error strings; key on 404 first, then (optional) message regex.
async getFeeAllowanceForGranterAndGrantee(granter: string, grantee: string): Promise<FeeAllowance | undefined> { try { const response = extractData(await this.httpClient.get<FeeAllowanceResponse>(`cosmos/feegrant/v1beta1/allowance/${granter}/${grantee}`)); return this.isValidFeeAllowance(response.allowance) ? response.allowance : undefined; } catch (error) { - if (isHttpError(error) && error.response?.data.message?.includes("fee-grant not found")) { - return undefined; - } + if (isHttpError(error)) { + if (error.response?.status === 404) return undefined; + const msg = error.response?.data?.message; + if (typeof msg === "string" && /fee-?grant not found/i.test(msg)) return undefined; + } throw error; } }
92-95: Specify radix in parseInt.Avoid implicit radix; be explicit (10).
- total: parseInt(allowances.pagination?.total || "0") + total: parseInt(allowances.pagination?.total || "0", 10)- total: parseInt(grants.pagination?.total || "0") + total: parseInt(grants.pagination?.total || "0", 10)Also applies to: 201-205
208-214: More robust date handling (invalid strings, TZ).Use parseISO and isValid to avoid surprises; aligns with date‑fns v4.1.0 behavior. Based on learnings.
-import { isFuture } from "date-fns"; +import { isFuture, parseISO, isValid as isValidDate } from "date-fns"; @@ - private isValidFeeAllowance(allowance: FeeAllowance) { - return !allowance.allowance.expiration || isFuture(new Date(allowance.allowance.expiration)); - } + private isValidFeeAllowance(allowance: FeeAllowance) { + const exp = allowance.allowance.expiration; + if (!exp) return true; + const dt = parseISO(exp); + return isValidDate(dt) && isFuture(dt); + } @@ - private isValidDepositDeploymentGrant(grant: ExactDepositDeploymentGrant) { - return this.isDepositDeploymentGrant(grant) && (!grant.expiration || isFuture(new Date(grant.expiration))); - } + private isValidDepositDeploymentGrant(grant: ExactDepositDeploymentGrant) { + if (!this.isDepositDeploymentGrant(grant)) return false; + if (!grant.expiration) return true; + const dt = parseISO(grant.expiration); + return isValidDate(dt) && isFuture(dt); + }
111-133: DRY: two nearly identical grant fetchers.Both methods hit the same endpoint and differ only by predicate. Consider a private fetchGrants(granter, grantee) to reduce duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/src/provider/services/provider/provider.service.ts(2 hunks)apps/deploy-web/src/queries/useBalancesQuery.ts(1 hunks)packages/http-sdk/src/authz/authz-http.service.ts(8 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/deploy-web/src/queries/useBalancesQuery.tspackages/http-sdk/src/authz/authz-http.service.tsapps/api/src/provider/services/provider/provider.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/deploy-web/src/queries/useBalancesQuery.tspackages/http-sdk/src/authz/authz-http.service.tsapps/api/src/provider/services/provider/provider.service.ts
🧬 Code graph analysis (2)
packages/http-sdk/src/authz/authz-http.service.ts (2)
packages/http-sdk/src/index.ts (1)
HttpClient(26-26)packages/http-sdk/src/http/http.service.ts (2)
extractData(12-14)extractData(17-19)
apps/api/src/provider/services/provider/provider.service.ts (3)
apps/indexer/drizzle/schema.ts (1)
provider(205-237)apps/api/src/provider/services/provider/provider-proxy.service.ts (1)
ProviderIdentity(7-10)apps/api/src/deployment/http-schemas/lease.schema.ts (1)
LeaseStatusResponse(53-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (4)
apps/api/src/provider/services/provider/provider.service.ts (2)
39-46: Resolved: 404 on missing provider is correctly surfacedSwitch to DB lookup +
assert(provider, 404, ...)fixes the earlier 500→404 issue. LGTM.
36-46: AllsendManifestandgetLeaseStatuscalls in the API code now match the updated signatures; no further changes required.apps/deploy-web/src/queries/useBalancesQuery.ts (1)
13-13: LGTM; readiness gating switched to isReady.Looks consistent with the new DI. Verify that balances fetching truly depends on authz readiness (and not a different client) to avoid unnecessary gating.
packages/http-sdk/src/authz/authz-http.service.ts (1)
5-5: Ignore import path suggestion: HttpClient is exclusively exported from src/utils/httpClient.ts and every service already imports it correctly from “../utils/httpClient”; there is no “../http/http-client” module.Likely an incorrect or invalid review comment.
stalniy
force-pushed
the
refactor/chain-api-clients
branch
from
48bfc3a to
1eb4342
Compare
2 participants
Why
to ensure that blockchain related services will fallback to indexerHttpClient and eventually fail with 404. ref #1950
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests