fix: ensure usesr profile request does not error in case of 401#2490
fix: ensure usesr profile request does not error in case of 401#2490
Conversation
📝 WalkthroughWalkthroughRefactors authentication/session handling to use service-based lazy session retrieval ( Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant NextJS_Page as Next.js (getServerSideProps)
participant DefineSSP as defineServerSideProps
participant AppServices as Services.getSession
participant PageGuard as redirectIfAccessTokenExpired
participant UrlService
Browser->>NextJS_Page: Request page
NextJS_Page->>DefineSSP: invoke defineServerSideProps
DefineSSP->>AppServices: getCurrentSession() (lazy)
AppServices-->>DefineSSP: Session|null
DefineSSP->>PageGuard: check accessToken expiry
alt token expired
PageGuard->>UrlService: newLogin(resolvedUrl)
PageGuard-->>NextJS_Page: { redirect: loginUrl }
else token valid
PageGuard-->>NextJS_Page: true (allow)
end
NextJS_Page-->>Browser: redirect or render page
sequenceDiagram
participant Client as Browser (client-side)
participant RequiredContainer
participant Services as UrlService
participant Router
Client->>RequiredContainer: renders
RequiredContainer->>Services: resolve urlService.newLogin()
alt user not authenticated
RequiredContainer->>Router: router.push(newLoginUrl)
else authenticated
RequiredContainer-->>Client: render children
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
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
Comment |
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (79.59%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2490 +/- ##
==========================================
- Coverage 50.97% 50.70% -0.28%
==========================================
Files 1066 1055 -11
Lines 29459 29109 -350
Branches 6535 6489 -46
==========================================
- Hits 15018 14760 -258
+ Misses 14095 13949 -146
- Partials 346 400 +54
*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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts (1)
67-71: Unit mismatch:accessTokenExpiresAtshould be in seconds, not milliseconds.The
accessTokenExpiresAtvalue at line 69 is in milliseconds (Date.now() + 1000 * 60 * 60 * 24 * 30), but the implementation atpageGuards.tsline 17 multiplies by 1000, expecting the value to be in seconds. This test passes by accident because the value is so large it remains in the future, but it's inconsistent with the expired token test at line 81 which correctly uses seconds.🐛 Proposed fix
it("returns true when access token is not expired", async () => { const context = setup({ session: { - accessTokenExpiresAt: Date.now() + 1000 * 60 * 60 * 24 * 30 + accessTokenExpiresAt: (Date.now() + 1000 * 60 * 60 * 24 * 30) / 1000 } });
🤖 Fix all issues with AI agents
In `@apps/deploy-web/src/components/user/RequiredUserContainer.tsx`:
- Around line 25-29: The useEffect in RequiredUserContainer uses
urlService.newLogin() but urlService is not listed in the dependency array,
violating exhaustive-deps; update the effect dependencies to include urlService
(i.e., change the dependency array from [user, isLoading, router] to [user,
isLoading, router, urlService]) so React correctly re-runs the effect when
urlService changes and remove the lint warning.
In `@apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx`:
- Around line 58-79: The mock for getProfile is returning { status: 401 } which
becomes { data: { status: 401 } } inside setup, so response.status is undefined
and the 401 branch in the real getProfile logic is never exercised; fix by
making test mocks return a full axios-like response shape (e.g.,
Promise.resolve({ status: 401, data: {} })) so getProfile sees response.status
at the top level, and update all other tests that mock getProfile in this file
(and any shared setup helpers) to return the same full response shape;
alternatively, if you prefer changing the helper, update setup to detect and
normalize nested data.status into a top-level status before passing to
useUser/getProfile.
🧹 Nitpick comments (2)
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (1)
19-19: Consider using a more specific type instead ofany.The
ifoption usesGetServerSidePropsResult<any>which violates the coding guidelines that discourageanyusage. Consider using a generic type parameter orunknownto maintain type safety.💡 Suggested improvement
- if?: (context: AppTypedContext<TSchema, TParams, TPreviewData>) => boolean | GetServerSidePropsResult<any> | Promise<GetServerSidePropsResult<any> | boolean>; + if?: (context: AppTypedContext<TSchema, TParams, TPreviewData>) => boolean | GetServerSidePropsResult<TProps> | Promise<GetServerSidePropsResult<TProps> | boolean>;apps/deploy-web/src/components/user/UserProviders/UserProviders.tsx (1)
15-22: Consider handling other 4xx status codes.The
validateStatusallows all status codes < 500 to pass without throwing, but only 401 is explicitly handled. If other client errors occur (403, 404, etc.),response.datawould be returned and might be an error object rather than valid user profile data, potentially causing issues downstream.If only 401 is expected in practice, this is fine. Otherwise, consider narrowing the check:
♻️ Suggested refinement
const getProfile = useCallback( async (url: string) => { - const response = await internalApiHttpClient.get(url, { validateStatus: status => status < 500 }); - if (response.status === 401) return; + const response = await internalApiHttpClient.get(url, { validateStatus: status => status < 400 || status === 401 }); + if (response.status === 401) return; return response.data; }, [internalApiHttpClient] );
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/deploy-web/env/.envapps/deploy-web/src/components/user/RequiredUserContainer.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.tsxapps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.tsapps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.tsapps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.tsapps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.tsapps/deploy-web/src/pages/api/auth/[...auth0].tsapps/deploy-web/src/pages/payment.tsxapps/deploy-web/src/pages/user/settings/favorites/index.tsxapps/deploy-web/src/pages/user/settings/index.tsxapps/deploy-web/src/services/app-di-container/app-di-container.tsapps/deploy-web/src/utils/withCustomPageAuthRequired.ts
💤 Files with no reviewable changes (2)
- apps/deploy-web/src/utils/withCustomPageAuthRequired.ts
- apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/deploy-web/src/pages/payment.tsxapps/deploy-web/src/pages/api/auth/[...auth0].tsapps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.tsapps/deploy-web/src/pages/user/settings/index.tsxapps/deploy-web/src/services/app-di-container/app-di-container.tsapps/deploy-web/src/components/user/RequiredUserContainer.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.tsxapps/deploy-web/src/pages/user/settings/favorites/index.tsxapps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.tsapps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsxapps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts
{apps/deploy-web,apps/provider-console}/**/*.spec.tsx
📄 CodeRabbit inference engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations
Files:
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
setupfunction instead ofbeforeEachin unit and service level tests
Files:
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsxapps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts
🧠 Learnings (8)
📚 Learning: 2025-08-12T13:52:38.708Z
Learnt from: stalniy
Repo: akash-network/console PR: 1800
File: apps/deploy-web/next.config.js:163-165
Timestamp: 2025-08-12T13:52:38.708Z
Learning: In the Akash Console project, akashnetwork/env-loader is used at the top of next.config.js files to automatically load environment variables from env/.env files into process.env. SENTRY_ORG and SENTRY_PROJECT are stored as public configuration values in apps/deploy-web/env/.env and are loaded this way, while only SENTRY_AUTH_TOKEN is handled as a GitHub secret in workflows.
Applied to files:
apps/deploy-web/env/.env
📚 Learning: 2025-08-12T13:52:38.708Z
Learnt from: stalniy
Repo: akash-network/console PR: 1800
File: apps/deploy-web/next.config.js:163-165
Timestamp: 2025-08-12T13:52:38.708Z
Learning: In the Akash Console project, SENTRY_ORG and SENTRY_PROJECT are stored as public configuration values in apps/deploy-web/env/.env file and loaded by akashnetwork/env-loader, not as GitHub secrets. Only SENTRY_AUTH_TOKEN should be handled as a secret.
Applied to files:
apps/deploy-web/env/.env
📚 Learning: 2025-07-11T10:46:43.711Z
Learnt from: stalniy
Repo: akash-network/console PR: 1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.711Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Applied to files:
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
📚 Learning: 2025-11-25T17:45:49.180Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-11-25T17:45:49.180Z
Learning: Applies to {apps/deploy-web,apps/provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations
Applied to files:
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsxapps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts
📚 Learning: 2025-11-25T17:45:44.790Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-11-25T17:45:44.790Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test
Applied to files:
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsxapps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
📚 Learning: 2026-01-13T17:40:26.436Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T17:40:26.436Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Use `setup` function instead of `beforeEach` in unit and service level tests
Applied to files:
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsxapps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts
📚 Learning: 2025-11-25T17:45:52.965Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-11-25T17:45:52.965Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files. The `setup` function must be at the bottom of the root `describe` block, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
Applied to files:
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsxapps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts
🧬 Code graph analysis (7)
apps/deploy-web/src/pages/payment.tsx (8)
apps/deploy-web/src/pages/user/settings/index.tsx (1)
getServerSideProps(12-15)apps/deploy-web/src/pages/profile/[username]/index.tsx (1)
getServerSideProps(19-38)apps/deploy-web/src/pages/template/[id]/index.tsx (1)
getServerSideProps(19-49)apps/deploy-web/src/pages/providers/[owner]/index.tsx (1)
getServerSideProps(19-40)apps/deploy-web/src/pages/alerts/index.tsx (1)
getServerSideProps(8-11)apps/deploy-web/src/pages/deployments/[dseq]/index.tsx (1)
getServerSideProps(8-22)apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (1)
defineServerSideProps(43-94)apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.ts (1)
redirectIfAccessTokenExpired(15-34)
apps/deploy-web/src/pages/api/auth/[...auth0].ts (1)
apps/deploy-web/src/services/app-di-container/server-di-container.service.ts (1)
services(25-61)
apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.ts (2)
apps/api/src/core/services/execution-context/execution-context.service.ts (1)
context(18-26)apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (1)
AppTypedContext(23-30)
apps/deploy-web/src/components/user/RequiredUserContainer.tsx (1)
apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(33-35)
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx (4)
apps/deploy-web/src/hooks/useUser.ts (1)
useUser(6-20)apps/deploy-web/src/types/user.ts (1)
CustomUserProfile(19-19)apps/deploy-web/src/services/user-tracker/user-tracker.service.ts (1)
UserTracker(5-19)apps/deploy-web/src/components/user/UserProviders/UserProviders.tsx (1)
UserProviders(13-30)
apps/deploy-web/src/pages/user/settings/favorites/index.tsx (3)
apps/deploy-web/src/pages/payment.tsx (1)
getServerSideProps(345-348)apps/deploy-web/src/pages/user/settings/index.tsx (1)
getServerSideProps(12-15)apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.ts (1)
redirectIfAccessTokenExpired(15-34)
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (1)
apps/api/src/core/services/execution-context/execution-context.service.ts (1)
context(18-26)
⏰ 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). (3)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (16)
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (2)
53-57: Good use ofoncefor lazy session evaluation.The
oncewrapper from lodash ensures that session retrieval is memoized per request, preventing redundant calls when multiple guards or handlers access the session. This is an efficient pattern for request-scoped caching.
72-74: Guard result handling logic is correct.The conditional check properly handles the three expected return types from the
ifguard:
- Object results (redirects, notFound) are returned directly
falsereturns NOT_FOUNDtrueorundefinedcontinues to handlerapps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.ts (2)
15-33: Return type change fromnulltotruealigns well with the guard pattern.The updated return type
Promise<{ redirect: Redirect } | true>is cleaner and more semantically meaningful than returningnullfor the success case. ThedefineServerSidePropshandler correctly interpretstrueto continue processing.
16-17: Session null handling defaults to expired token correctly.When
sessionisnull, the fallback(session?.accessTokenExpiresAt || 0) * 1_000correctly results in epoch time (Jan 1, 1970), which will always be less thannew Date(). This ensures unauthenticated users are redirected to login, which is the expected behavior.apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts (1)
97-107: Test setup follows coding guidelines correctly.The
setupfunction is positioned at the bottom of the rootdescribeblock, usesjest-mock-extendedfor mocking, accepts a single parameter with inline type definition, and avoids shared state. The mock forgetCurrentSessioncorrectly returns a Promise.Based on learnings, this follows the required pattern.
apps/deploy-web/src/pages/payment.tsx (1)
345-348: Clean migration to the new authentication guard pattern.The replacement of
withCustomPageAuthRequiredwithdefineServerSidePropsusing theredirectIfAccessTokenExpiredguard is consistent with the PR's objective. This approach provides better error handling for 401 responses and reduces unnecessary Sentry errors.apps/deploy-web/src/pages/user/settings/favorites/index.tsx (1)
11-14: Consistent implementation of the new auth guard pattern.The migration follows the same pattern established in other pages (payment.tsx, user/settings/index.tsx). The route string correctly reflects the page path.
apps/deploy-web/src/pages/user/settings/index.tsx (2)
4-4: LGTM!Clean import of the new guard function that aligns with the refactored authentication pattern.
12-15: LGTM!The declarative approach using
defineServerSidePropswith theredirectIfAccessTokenExpiredguard cleanly handles the authentication check. This aligns well with the PR objective of gracefully handling 401 responses without producing errors.apps/deploy-web/src/components/user/RequiredUserContainer.tsx (1)
6-6: LGTM!The import and usage of
useServicesto retrieveurlServiceis correct and aligns with the service-based architecture pattern in the codebase.Also applies to: 23-23
apps/deploy-web/env/.env (1)
37-37: ✓ LGTM! New login path configuration aligns with the updated auth flow.The addition of
NEXT_PUBLIC_AUTH0_LOGINprovides a configurable login path for client-side redirects, which aligns with the PR's goal of improving the authentication flow user experience. The/loginroute is properly defined atpages/login/index.tsxand is actively used throughout the codebase via theurlService.newLogin()method.apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx (1)
81-99: Setup function structure follows project conventions.The setup function is properly positioned at the bottom of the describe block, accepts a single parameter with inline type definition, and avoids shared state. The
Contentparameter extension allows flexible testing of child component behavior.apps/deploy-web/src/services/app-di-container/app-di-container.ts (2)
162-168: Good retry policy for server errors.The retry logic correctly limits retries to HTTP 5xx errors only, avoiding unnecessary retries for client errors (4xx including 401). This aligns well with the PR objective of reducing Sentry noise from authentication failures while maintaining resilience against transient server issues.
Note: With
failureCount < 3, queries will make up to 3 total attempts (original request + 2 retries).
7-7: LGTM on the import addition.The
isHttpErrorimport is appropriately added to support the new retry logic and is correctly exported from@akashnetwork/http-sdk.apps/deploy-web/src/pages/api/auth/[...auth0].ts (2)
8-8: LGTM on import refactoring.Correctly removed direct
getSessionimport since it's now accessed via the services container, maintaining only the error types needed locally.
74-74: Proper migration to service-based session retrieval.The change from direct
getSession(req, res)toservices.getSession(req, res)aligns with the centralized DI pattern shown inserver-di-container.service.tswheregetSession: () => getSessionis registered. This improves testability and maintains consistency with the broader architectural refactoring in this PR.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
Show resolved
Hide resolved
stalniy
force-pushed
the
fix/user-unauthenticated
branch
from
210f169 to
e6f7bbc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx`:
- Around line 62-65: The AxiosResponse generic is incorrect: change
AxiosResponse<{ data: CustomUserProfile } | undefined> to
AxiosResponse<CustomUserProfile | undefined> so mocks reflect that getProfile
returns response.data (the CustomUserProfile). Update the test/mock where
Promise.resolve(...) is used and also adjust the test helper/setup function
signature (the setup function referenced in the spec) to accept/expect
CustomUserProfile | undefined instead of the nested shape so types align with
getProfile and the Axios mock.
🧹 Nitpick comments (1)
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx (1)
94-96: Default mock missingstatusproperty for consistency.Other mocks (lines 33-34, 62-65) include the
statusproperty, but the default fallback doesn't. This inconsistency could cause unexpected behavior if a test relies on the default and the implementation checksresponse.status.🔧 Proposed fix
if (input?.getProfile) return input.getProfile(); return Promise.resolve({ + status: 200, data: buildUser() });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/deploy-web/env/.envapps/deploy-web/src/components/user/RequiredUserContainer.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.tsxapps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.tsapps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.tsapps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.tsapps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.tsapps/deploy-web/src/pages/api/auth/[...auth0].tsapps/deploy-web/src/pages/payment.tsxapps/deploy-web/src/pages/user/settings/favorites/index.tsxapps/deploy-web/src/pages/user/settings/index.tsxapps/deploy-web/src/services/app-di-container/app-di-container.tsapps/deploy-web/src/utils/withCustomPageAuthRequired.ts
💤 Files with no reviewable changes (2)
- apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts
- apps/deploy-web/src/utils/withCustomPageAuthRequired.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/deploy-web/src/pages/user/settings/index.tsx
- apps/deploy-web/env/.env
- apps/deploy-web/src/services/app-di-container/app-di-container.ts
- apps/deploy-web/src/components/user/RequiredUserContainer.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/deploy-web/src/pages/api/auth/[...auth0].tsapps/deploy-web/src/components/user/UserProviders/UserProviders.tsxapps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.tsapps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.tsapps/deploy-web/src/pages/payment.tsxapps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.tsapps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsxapps/deploy-web/src/pages/user/settings/favorites/index.tsx
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.tsapps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
setupfunction instead ofbeforeEachin unit and service level tests
Files:
apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.tsapps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
{apps/deploy-web,apps/provider-console}/**/*.spec.tsx
📄 CodeRabbit inference engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations
Files:
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
🧠 Learnings (6)
📚 Learning: 2025-11-25T17:45:49.180Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-11-25T17:45:49.180Z
Learning: Applies to {apps/deploy-web,apps/provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations
Applied to files:
apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.tsapps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
📚 Learning: 2025-07-11T10:46:43.711Z
Learnt from: stalniy
Repo: akash-network/console PR: 1660
File: apps/deploy-web/src/components/alerts/DeploymentAlertsContainer/DeploymentAlertsContainer.spec.tsx:54-56
Timestamp: 2025-07-11T10:46:43.711Z
Learning: In apps/{deploy-web,provider-console}/**/*.spec.tsx files: Use `getBy` methods instead of `queryBy` methods when testing element presence with `toBeInTheDocument()` because `getBy` throws an error and shows DOM state when element is not found, providing better debugging information than `queryBy` which returns null.
Applied to files:
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
📚 Learning: 2025-11-25T17:45:44.790Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-11-25T17:45:44.790Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test
Applied to files:
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
📚 Learning: 2025-11-19T15:15:07.283Z
Learnt from: ygrishajev
Repo: akash-network/console PR: 2254
File: apps/api/test/functional/sign-and-broadcast-tx.spec.ts:4-4
Timestamp: 2025-11-19T15:15:07.283Z
Learning: In the Akash Network Console project, when tests use native Node.js fetch (available in Node 18+), fetch-mock should be used for HTTP mocking instead of nock, as nock does not support intercepting native fetch calls. This applies to apps/api/test/functional/sign-and-broadcast-tx.spec.ts and any other tests using native fetch.
Applied to files:
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
📚 Learning: 2026-01-13T17:40:26.436Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-13T17:40:26.436Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Use `setup` function instead of `beforeEach` in unit and service level tests
Applied to files:
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
📚 Learning: 2025-11-25T17:45:52.965Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-11-25T17:45:52.965Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files. The `setup` function must be at the bottom of the root `describe` block, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
Applied to files:
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
🧬 Code graph analysis (4)
apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.ts (2)
apps/api/src/core/services/execution-context/execution-context.service.ts (1)
context(18-26)apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (1)
AppTypedContext(23-30)
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (1)
apps/api/src/core/services/execution-context/execution-context.service.ts (1)
context(18-26)
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx (3)
apps/deploy-web/src/types/user.ts (1)
CustomUserProfile(19-19)apps/deploy-web/src/hooks/useUser.ts (1)
useUser(6-20)apps/deploy-web/src/components/user/UserProviders/UserProviders.tsx (1)
UserProviders(13-30)
apps/deploy-web/src/pages/user/settings/favorites/index.tsx (4)
apps/deploy-web/src/pages/payment.tsx (1)
getServerSideProps(345-348)apps/deploy-web/src/pages/user/settings/index.tsx (1)
getServerSideProps(12-15)apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (1)
defineServerSideProps(43-94)apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.ts (1)
redirectIfAccessTokenExpired(15-34)
⏰ 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). (3)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (12)
apps/deploy-web/src/pages/api/auth/[...auth0].ts (2)
7-9: LGTM!The import change correctly removes the direct
getSessionimport while retainingAccessTokenErrorandAccessTokenErrorCodewhich are still needed for theisInvalidSessionErrorfunction.
71-79: LGTM!The switch to
services.getSession(req, res)aligns well with the DI pattern used throughout the codebase. The subsequent null check gracefully handles unauthenticated users by returning a clean 401 JSON response rather than throwing an error, which addresses the PR objective of reducing Sentry noise for expected unauthenticated states.apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts (2)
66-76: LGTM!The test correctly verifies that
redirectIfAccessTokenExpiredreturnstruewhen the access token is not expired, aligning with the updated function signature that returnsPromise<{ redirect: Redirect } | true>.
97-107: LGTM!The
setupfunction follows the coding guidelines: placed at the bottom of the rootdescribeblock, accepts a single parameter with an inline type definition, usesjest-mock-extendedfor mocking, and avoids shared state.apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (2)
53-57: LGTM!Using
once()to wrapgetCurrentSessionis a good pattern for lazy evaluation and memoization. This ensures the session is fetched only once per request, even if multiple guards or handlers callgetCurrentSession().
19-19: LGTM!The updated
ifsignature correctly supports both synchronous and asynchronous guards that can returnbooleanorGetServerSidePropsResult(for redirects/not-found responses).apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.ts (1)
15-34: LGTM!The refactored
redirectIfAccessTokenExpiredcorrectly handles the authentication flow:
- Returns
truefor valid sessions, allowing the request to proceed- Returns a redirect object for expired/missing sessions
- Properly converts
accessTokenExpiresAtfrom seconds to milliseconds- Uses structured logging for better observability
The distinction between
context.req.url(for logging the raw request) andcontext.resolvedUrl(for the redirect destination with resolved dynamic routes) is appropriate.apps/deploy-web/src/pages/payment.tsx (1)
345-348: LGTM!Clean migration from
withCustomPageAuthRequiredto the newdefineServerSidePropspattern withredirectIfAccessTokenExpiredguard. This aligns with the PR's goal of improving 401 handling.apps/deploy-web/src/pages/user/settings/favorites/index.tsx (1)
11-14: LGTM!Consistent application of the new
defineServerSidePropspattern withredirectIfAccessTokenExpiredguard, matching the approach used in other pages in this PR.apps/deploy-web/src/components/user/UserProviders/UserProviders.tsx (1)
15-22: Clean implementation for graceful 401 handling.The
validateStatuscallback ensures Axios won't throw for 4xx responses, and returningundefinedon 401 allows the Auth0 UserProvider to handle the unauthenticated state without triggering an error. TheuseCallbackmemoization with the correct dependency is appropriate.apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx (2)
59-81: Good test coverage for 401 handling.This test validates the core PR objective by confirming that a 401 response results in
userbeing undefined without triggering an error state. The use of a customContentcomponent to inspectuseUserstate is a clean approach.
31-34: Mock shape now correctly simulates Axios responses.The mocks properly return full response objects with
statusanddataproperties, ensuring the 401 handling logic ingetProfileis correctly exercised.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| Promise.resolve({ | ||
| status: 401, | ||
| data: undefined | ||
| } as AxiosResponse<{ data: CustomUserProfile } | undefined>) |
There was a problem hiding this comment.
Incorrect generic type for AxiosResponse.
The type AxiosResponse<{ data: CustomUserProfile } | undefined> suggests response.data.data would contain the user profile. However, getProfile returns response.data directly (the CustomUserProfile), not a nested structure.
🔧 Proposed fix
getProfile: jest.fn(() =>
Promise.resolve({
status: 401,
data: undefined
- } as AxiosResponse<{ data: CustomUserProfile } | undefined>)
+ } as AxiosResponse<CustomUserProfile | undefined>)
),Also update the setup function signature at line 84:
async function setup(input?: {
- getProfile?: () => Promise<AxiosResponse<{ data: CustomUserProfile } | undefined>>;
+ getProfile?: () => Promise<AxiosResponse<CustomUserProfile | undefined>>;
userTracker?: UserTracker;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Promise.resolve({ | |
| status: 401, | |
| data: undefined | |
| } as AxiosResponse<{ data: CustomUserProfile } | undefined>) | |
| Promise.resolve({ | |
| status: 401, | |
| data: undefined | |
| } as AxiosResponse<CustomUserProfile | undefined>) |
🤖 Prompt for AI Agents
In `@apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx`
around lines 62 - 65, The AxiosResponse generic is incorrect: change
AxiosResponse<{ data: CustomUserProfile } | undefined> to
AxiosResponse<CustomUserProfile | undefined> so mocks reflect that getProfile
returns response.data (the CustomUserProfile). Update the test/mock where
Promise.resolve(...) is used and also adjust the test helper/setup function
signature (the setup function referenced in the spec) to accept/expect
CustomUserProfile | undefined instead of the nested shape so types align with
getProfile and the Axios mock.
2 participants
Why
To improve auth flow UX and reduce amount of sentry errors
Summary by CodeRabbit
Bug Fixes
Improvements
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.