Skip to content

feat: improves observability of SSR part of deploy-web#1642

Merged
stalniy merged 1 commit intomainfrom
feat/observability-deploy-web-ssr
Jul 8, 2025
Merged

feat: improves observability of SSR part of deploy-web#1642
stalniy merged 1 commit intomainfrom
feat/observability-deploy-web-ssr

Conversation

@stalniy
Copy link
Contributor

@stalniy stalniy commented Jul 7, 2025

Why

To improve reaction on bugs

Summary by CodeRabbit

  • Refactor
    • Replaced direct console logging with structured logging and centralized error reporting across the app, including API endpoints and middleware.
    • Enhanced error handling to classify errors by severity and add metadata for better categorization and traceability.
    • Updated error handling services to support structured error contexts, logging integration, and tracing data.
    • Modified feature flag service to use dependency injection with per-instance creation instead of a shared singleton.
    • Refactored server-side props fetching in key pages to use a new abstraction with integrated validation, guards, and services.
    • Updated route protection and session retrieval to use centralized services for consistency.
  • New Features
    • Introduced a utility to simplify Next.js server-side props creation with enhanced type safety, validation, and error tracking.
    • Added asynchronous page guard utilities for feature flag checks, user registration status, and access token expiration redirects.
  • Tests
    • Improved and expanded tests for error handling, including new coverage for callback wrapping and logger integration.
    • Added comprehensive tests for server-side props utility and page guard functions.

@stalniy stalniy requested a review from a team as a code owner July 7, 2025 14:00
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 7, 2025

Walkthrough

This change replaces direct console logging and ad-hoc error handling throughout the deploy-web app with a centralized, structured error handling and logging system. It introduces a new error handler service that supports severity levels, tags, and tracing, and updates all relevant modules and API endpoints to use this system. Tests are enhanced to cover the new functionality. Additionally, new Next.js server-side props utilities and page guard functions are added, feature flag service instantiation is refactored, and deprecated server-side props utilities are marked as such.

Changes

File(s) Change Summary
src/instrumentation.ts, src/middleware.ts, src/pages/api/proxy/[...path].ts Replaced console logging with structured logging via centralized logger service (di.logger or services.logger).
src/pages/api/auth/[...auth0].ts, src/pages/api/auth/signup.ts Updated error handling to use centralized error handler with severity and tags; removed direct console logging.
src/pages/api/bitbucket/authenticate.ts, src/pages/api/bitbucket/refresh.ts Changed error logging to use error handler service with structured tags; removed console logs.
src/pages/api/github/authenticate.ts, src/pages/api/gitlab/authenticate.ts Updated error handling to use error handler with tags for provider and event; removed console logs.
src/pages/api/gitlab/refresh.ts Changed error reporting to use centralized error handler with structured tags.
src/services/app-di-container/app-di-container.ts Modified error handler instantiation to inject logger; updated query/mutation error callbacks to use reportError.
src/services/error-handler/error-handler.service.ts Major enhancement: added structured error reporting, severity levels, tags, tracing, and callback wrapping utility.
src/services/error-handler/error-handler.service.spec.ts Enhanced and expanded tests for new error handler functionality, including logger integration and callback wrapping.
src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts Added new utility to define typed, validated, and context-aware Next.js server-side props handlers with error tracking.
src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts Added comprehensive tests for defineServerSideProps covering validation, conditionals, redirects, and error cases.
src/lib/nextjs/pageGuards/pageGuards.ts Added new page guard utilities for feature flag checks, user registration status, and access token expiration redirects.
src/lib/nextjs/pageGuards/pageGuards.spec.ts Added tests for page guard functions verifying feature flag checks, user session presence, and redirect logic.
src/lib/nextjs/getServerSidePropsWithServices.ts, src/lib/nextjs/getValidatedServerSideProps.ts Added deprecation notices recommending use of defineServerSideProps instead.
src/services/feature-flag/index.ts Removed deprecated singleton feature flag service export.
src/services/http/http-server.service.ts Changed feature flag service import to class and instantiate new instance per container creation instead of singleton.
src/pages/alerts/notification-channels/[id]/index.ts(x) Refactored server-side props to use defineServerSideProps with schema validation, guards, and services from context.
next.config.js Added "@panva/hkdf" and "jose" to transpilePackages for test environment.
src/services/route-protector/index.ts Updated routeProtector initialization to use centralized services imports instead of direct imports.

Sequence Diagram(s)

sequenceDiagram
    participant API_Handler as API Handler
    participant Logger as LoggerService
    participant ErrorHandler as ErrorHandlerService
    participant Sentry as Sentry

    API_Handler->>Logger: logger.info({ ... })
    API_Handler->>ErrorHandler: reportError({ error, severity, tags, extra })
    ErrorHandler->>Logger: logger.error({ ... })
    ErrorHandler->>Sentry: captureException(error, { tags, extra, severity })
    Note over API_Handler,ErrorHandler: API handler responds to client based on error severity
Loading

Possibly related PRs

Suggested reviewers

  • baktun14

Poem

🐇 In code's deep burrow, logs took flight,
Errors caught with tags so bright.
From console's shout to structured song,
Our handlers keep the system strong.
Features guarded, props defined,
In this warren, peace we find!
🌿✨

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

npm error Exit handler never called!
npm error This is an error with npm itself. Please report this error at:
npm error https://github.com/npm/cli/issues
npm error A complete log of this run can be found in: /.npm/_logs/2025-07-08T16_35_08_922Z-debug-0.log

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codecov
Copy link

codecov bot commented Jul 7, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 53 lines in your changes missing coverage. Please review.

Project coverage is 44.68%. Comparing base (175b31c) to head (f1c1df2).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
apps/deploy-web/src/pages/api/auth/[...auth0].ts 0.00% 8 Missing ⚠️
.../pages/alerts/notification-channels/[id]/index.tsx 0.00% 7 Missing ⚠️
apps/deploy-web/src/pages/api/auth/signup.ts 0.00% 7 Missing ⚠️
apps/deploy-web/src/middleware.ts 0.00% 5 Missing ⚠️
apps/deploy-web/src/instrumentation.ts 0.00% 4 Missing ⚠️
apps/deploy-web/src/pages/api/proxy/[...path].ts 0.00% 3 Missing ⚠️
.../src/services/app-di-container/app-di-container.ts 0.00% 3 Missing ⚠️
...rc/services/error-handler/error-handler.service.ts 87.50% 3 Missing ⚠️
...deploy-web/src/pages/api/bitbucket/authenticate.ts 0.00% 2 Missing ⚠️
apps/deploy-web/src/pages/api/bitbucket/refresh.ts 0.00% 2 Missing ⚠️
... and 5 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1642      +/-   ##
==========================================
+ Coverage   44.43%   44.68%   +0.25%     
==========================================
  Files         885      886       +1     
  Lines       21708    21750      +42     
  Branches     3964     3950      -14     
==========================================
+ Hits         9645     9719      +74     
+ Misses      11396    11368      -28     
+ Partials      667      663       -4     
Flag Coverage Δ *Carryforward flag
api 80.07% <ø> (ø) Carriedforward from 175b31c
deploy-web 20.49% <50.00%> (+0.49%) ⬆️
notifications 87.90% <ø> (ø) Carriedforward from 175b31c
provider-proxy 83.99% <ø> (ø) Carriedforward from 175b31c

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...tjs/defineServerSideProps/defineServerSideProps.ts 100.00% <100.00%> (ø)
...b/src/lib/nextjs/getServerSidePropsWithServices.ts 0.00% <ø> (ø)
...-web/src/lib/nextjs/getValidatedServerSideProps.ts 0.00% <ø> (ø)
...deploy-web/src/lib/nextjs/pageGuards/pageGuards.ts 100.00% <100.00%> (ø)
...eploy-web/src/services/http/http-server.service.ts 78.57% <66.66%> (+78.57%) ⬆️
...deploy-web/src/pages/api/bitbucket/authenticate.ts 0.00% <0.00%> (ø)
apps/deploy-web/src/pages/api/bitbucket/refresh.ts 0.00% <0.00%> (ø)
...ps/deploy-web/src/pages/api/github/authenticate.ts 0.00% <0.00%> (ø)
...ps/deploy-web/src/pages/api/gitlab/authenticate.ts 0.00% <0.00%> (ø)
apps/deploy-web/src/pages/api/gitlab/refresh.ts 0.00% <0.00%> (ø)
... and 9 more

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/deploy-web/src/services/error-handler/error-handler.service.ts (1)

42-43: Consider more robust promise detection.

While the current implementation works, checking typeof result.catch === "function" might miss some edge cases. Consider using result instanceof Promise for more explicit promise detection:

-        if (result && typeof result.catch === "function") {
+        if (result instanceof Promise) {

Alternatively, you could check for both then and catch methods for broader compatibility with promise-like objects.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 06d0279 and 9567fe2.

📒 Files selected for processing (13)
  • apps/deploy-web/src/instrumentation.ts (2 hunks)
  • apps/deploy-web/src/middleware.ts (2 hunks)
  • apps/deploy-web/src/pages/api/auth/[...auth0].ts (3 hunks)
  • apps/deploy-web/src/pages/api/auth/signup.ts (2 hunks)
  • apps/deploy-web/src/pages/api/bitbucket/authenticate.ts (2 hunks)
  • apps/deploy-web/src/pages/api/bitbucket/refresh.ts (2 hunks)
  • apps/deploy-web/src/pages/api/github/authenticate.ts (2 hunks)
  • apps/deploy-web/src/pages/api/gitlab/authenticate.ts (2 hunks)
  • apps/deploy-web/src/pages/api/gitlab/refresh.ts (2 hunks)
  • apps/deploy-web/src/pages/api/proxy/[...path].ts (2 hunks)
  • apps/deploy-web/src/services/app-di-container/app-di-container.ts (1 hunks)
  • apps/deploy-web/src/services/error-handler/error-handler.service.spec.ts (3 hunks)
  • apps/deploy-web/src/services/error-handler/error-handler.service.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.spec.{ts,tsx}`: Use `setup` function instead of `beforeEach` in test files...

**/*.spec.{ts,tsx}: Use setup function instead of beforeEach in test files
setup function must be at the bottom of the root describe block
setup function creates an object under test and returns it
setup function should accept a single parameter with inline type definition
Don't use shared state in setup function
Don't specify return type of setup function

📄 Source: CodeRabbit Inference Engine (.cursor/rules/setup-instead-of-before-each.mdc)

List of files the instruction was applied to:

  • apps/deploy-web/src/services/error-handler/error-handler.service.spec.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
Learnt from: jzsfkzm
PR: akash-network/console#1364
File: apps/api/src/provider/routes/provider-deployments/provider-deployments.router.ts:38-44
Timestamp: 2025-05-25T19:37:00.800Z
Learning: The Akash Console API uses centralized error handling through the OpenApiHonoHandler's defaultHook, which automatically processes errors via HonoErrorHandlerService. Individual route handlers do not use explicit try-catch blocks and instead rely on this framework-level error handling mechanism.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
apps/deploy-web/src/pages/api/github/authenticate.ts (1)
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
apps/deploy-web/src/pages/api/proxy/[...path].ts (2)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: jzsfkzm
PR: akash-network/console#1372
File: apps/api/src/dashboard/services/stats/stats.service.ts:0-0
Timestamp: 2025-05-28T20:42:58.200Z
Learning: The user successfully implemented CosmosHttpService with retry logic using axiosRetry, exponential backoff, and proper error handling for Cosmos API calls, replacing direct axios usage in StatsService.
apps/deploy-web/src/pages/api/gitlab/refresh.ts (1)
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
apps/deploy-web/src/pages/api/auth/[...auth0].ts (1)
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
apps/deploy-web/src/pages/api/auth/signup.ts (1)
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
apps/deploy-web/src/services/app-di-container/app-di-container.ts (2)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
apps/deploy-web/src/pages/api/gitlab/authenticate.ts (1)
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
apps/deploy-web/src/pages/api/bitbucket/authenticate.ts (1)
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
apps/deploy-web/src/pages/api/bitbucket/refresh.ts (1)
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
apps/deploy-web/src/services/error-handler/error-handler.service.spec.ts (7)
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
apps/deploy-web/src/services/error-handler/error-handler.service.ts (2)
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
Learnt from: jzsfkzm
PR: akash-network/console#1364
File: apps/api/src/provider/routes/provider-deployments/provider-deployments.router.ts:38-44
Timestamp: 2025-05-25T19:37:00.800Z
Learning: The Akash Console API uses centralized error handling through the OpenApiHonoHandler's defaultHook, which automatically processes errors via HonoErrorHandlerService. Individual route handlers do not use explicit try-catch blocks and instead rely on this framework-level error handling mechanism.
🧬 Code Graph Analysis (6)
apps/deploy-web/src/pages/api/github/authenticate.ts (2)
packages/logging/src/servicies/logger/logger.service.ts (1)
  • error (107-109)
apps/deploy-web/src/services/http/http-server.service.ts (1)
  • services (20-24)
apps/deploy-web/src/middleware.ts (1)
packages/logging/src/servicies/logger/logger.service.ts (1)
  • error (107-109)
apps/deploy-web/src/pages/api/proxy/[...path].ts (2)
apps/deploy-web/src/lib/nextjs/wrapApiHandler.ts (1)
  • wrapApiHandlerInExecutionContext (5-9)
apps/deploy-web/src/services/http/http-server.service.ts (1)
  • services (20-24)
apps/deploy-web/src/pages/api/gitlab/refresh.ts (2)
packages/logging/src/servicies/logger/logger.service.ts (1)
  • error (107-109)
apps/deploy-web/src/services/http/http-server.service.ts (1)
  • services (20-24)
apps/deploy-web/src/pages/api/auth/signup.ts (2)
apps/deploy-web/src/services/error-handler/error-handler.service.ts (1)
  • SeverityLevel (57-57)
apps/deploy-web/src/services/http/http-server.service.ts (1)
  • services (20-24)
apps/deploy-web/src/pages/api/gitlab/authenticate.ts (2)
packages/logging/src/servicies/logger/logger.service.ts (1)
  • error (107-109)
apps/deploy-web/src/services/http/http-server.service.ts (1)
  • services (20-24)
⏰ 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)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
🔇 Additional comments (27)
apps/deploy-web/src/instrumentation.ts (2)

3-3: LGTM: Proper dependency injection import

Clean import of the services container for centralized logging access.


23-23: LGTM: Structured logging improvement

Excellent replacement of console.error with structured logging. The structured format with message and error properties will improve observability and debugging capabilities.

apps/deploy-web/src/services/app-di-container/app-di-container.ts (3)

100-100: LGTM: Consistent error reporting API update

Good update to use the new reportError method with structured parameters for better error handling consistency.


103-103: LGTM: Consistent mutation cache error handling

Properly updated to match the query cache error handling pattern using the new reportError API.


106-106: LGTM: Logger dependency injection

Correctly updated ErrorHandlerService instantiation to inject the logger dependency, enabling structured logging within the error handler.

apps/deploy-web/src/pages/api/github/authenticate.ts (2)

6-6: LGTM: Centralized services import

Proper import of services container for accessing centralized error handling.


26-26: LGTM: Excellent structured error reporting

Outstanding improvement with rich contextual tags for error categorization. The tags (category: "auth", event: "AUTH_EXCHANGE_CODE_FOR_TOKENS_ERROR", provider: "github") will enable powerful filtering and analysis of authentication errors.

apps/deploy-web/src/middleware.ts (4)

4-4: LGTM: Dependency injection import

Clean import of services container for centralized logging access.


13-13: LGTM: Structured info logging

Good improvement to structured logging format for maintenance page redirects, enhancing observability.


18-18: LGTM: Consistent structured logging

Proper structured logging format maintained for redirect operations.


33-33: LGTM: Structured error logging

Excellent structured error logging with both message and error properties, providing better debugging context for URL parsing failures.

apps/deploy-web/src/pages/api/proxy/[...path].ts (3)

7-7: LGTM: Centralized services import

Proper import of services container for accessing centralized logging.


13-13: LGTM: Structured proxy request logging

Excellent structured logging with event name and URL for proxy requests. The PROXY_API_REQUEST event will enable better monitoring and analytics of proxy operations.


39-39: LGTM: Structured proxy error logging

Good structured error logging with event name and error details. The PROXY_API_REQUEST_ERROR event will facilitate better error tracking and debugging of proxy failures.

apps/deploy-web/src/pages/api/gitlab/authenticate.ts (2)

6-6: LGTM: Centralized error reporting import added.

The import of the services container enables access to the centralized error handler, improving observability as intended.


25-27: LGTM: Structured error reporting with appropriate tags.

The error handling now uses centralized reporting with well-structured tags for better observability:

  • category: "auth" - categorizes authentication-related errors
  • event: "AUTH_EXCHANGE_CODE_FOR_TOKENS_ERROR" - specific event identification
  • provider: "gitlab" - provider context for filtering

This enables better monitoring and debugging of authentication issues while maintaining the same user-facing error response.

apps/deploy-web/src/pages/api/gitlab/refresh.ts (2)

6-6: LGTM: Consistent import pattern for centralized services.

The services import follows the same pattern as other authentication endpoints, maintaining consistency across the codebase.


25-27: LGTM: Consistent error reporting for token refresh.

The error handling follows the same structured approach as other authentication endpoints, with appropriate event tagging (AUTH_REFRESH_TOKEN_ERROR) to distinguish refresh errors from authentication errors.

apps/deploy-web/src/pages/api/bitbucket/authenticate.ts (2)

6-6: LGTM: Consistent services import for error handling.

The import follows the established pattern across authentication endpoints.


25-27: LGTM: Consistent error reporting with correct provider tagging.

The error handling maintains consistency with other authentication endpoints while correctly identifying the provider as "bitbucket" for proper categorization and monitoring.

apps/deploy-web/src/pages/api/bitbucket/refresh.ts (2)

6-6: LGTM: Consistent pattern maintained across providers.

The services import follows the established pattern for centralized error handling.


26-28: LGTM: Consistent error reporting for Bitbucket token refresh.

The error handling maintains the same structured approach as other endpoints, with appropriate tagging to distinguish refresh token errors from authentication errors and identify the Bitbucket provider.

apps/deploy-web/src/pages/api/auth/signup.ts (2)

5-6: LGTM: Appropriate imports for enhanced error handling.

The imports enable both severity classification and centralized error reporting, supporting the more sophisticated error handling in this endpoint.


20-27: LGTM: Enhanced error handling with severity classification.

The error handling provides better observability and user experience:

  • Severity classification: Client errors (4xx) get "warning" severity vs "error" for others
  • Appropriate HTTP responses: 400 for client errors, 503 for server errors
  • Structured reporting: Consistent tagging with category: "auth0" and event: "AUTH_SIGNUP_ERROR"

This approach provides better error categorization for monitoring while giving users more appropriate feedback based on error type.

apps/deploy-web/src/pages/api/auth/[...auth0].ts (1)

8-9: LGTM! Excellent improvement to observability.

The changes successfully replace console logging with structured logging and centralized error handling, which aligns perfectly with the PR objective of improving SSR observability. The error severity classification based on HTTP status codes (client errors as warnings, server errors as errors) is a thoughtful touch that will help with monitoring and alerting.

This implementation is consistent with the centralized error handling pattern mentioned in the retrieved learnings.

Also applies to: 39-39, 70-72, 77-86

apps/deploy-web/src/services/error-handler/error-handler.service.spec.ts (1)

1-189: Excellent test coverage and adherence to coding guidelines!

The test file properly follows all the coding guidelines for test files, with the setup function correctly placed at the bottom of the root describe block. The updated tests provide comprehensive coverage for the refactored ErrorHandlerService, including:

  • Proper mocking and injection of the LoggerService
  • Updated tests using the new reportError method
  • Thorough coverage of the new wrapCallback method for both sync and async functions
  • Testing of fallback value functionality
apps/deploy-web/src/services/error-handler/error-handler.service.ts (1)

1-74: Outstanding refactoring that significantly improves error handling and observability!

The refactored ErrorHandlerService provides:

  • Structured logging integration via LoggerService
  • Distributed tracing support with getTraceData()
  • Comprehensive error reporting with severity levels and tags
  • A powerful wrapCallback utility for safe function execution

The implementation is clean, well-typed, and follows SOLID principles.

@stalniy stalniy force-pushed the feat/observability-deploy-web-ssr branch from 9567fe2 to 2d9490f Compare July 7, 2025 16:15
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (2)

48-52: Consider safer context merging and type assertion.

The context merging and type assertion could be made safer:

  1. The spread operator might override important context properties
  2. The as assertion bypasses type checking
-      const newContext = {
-        ...context,
-        ...validatedContext,
-        services
-      } as AppTypedContext<TSchema, TParams, TPreviewData>;
+      const newContext: AppTypedContext<TSchema, TParams, TPreviewData> = {
+        ...context,
+        ...(validatedContext || {}),
+        services
+      };

This ensures safer handling of undefined validatedContext and provides better type safety.


58-62: Remove unsafe type assertion in fallback case.

The fallback return uses as any which bypasses type checking. Consider providing a more type-safe alternative.

-      return { props: {} } as any;
+      return { props: {} as TProps };

This maintains type safety while still providing the empty props fallback.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9567fe2 and 2d9490f.

📒 Files selected for processing (22)
  • apps/deploy-web/next.config.js (1 hunks)
  • apps/deploy-web/src/instrumentation.ts (2 hunks)
  • apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts (1 hunks)
  • apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (1 hunks)
  • apps/deploy-web/src/lib/nextjs/getServerSidePropsWithServices.ts (1 hunks)
  • apps/deploy-web/src/lib/nextjs/getValidatedServerSideProps.ts (1 hunks)
  • apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts (1 hunks)
  • apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.ts (1 hunks)
  • apps/deploy-web/src/middleware.ts (2 hunks)
  • apps/deploy-web/src/pages/api/auth/[...auth0].ts (3 hunks)
  • apps/deploy-web/src/pages/api/auth/signup.ts (2 hunks)
  • apps/deploy-web/src/pages/api/bitbucket/authenticate.ts (2 hunks)
  • apps/deploy-web/src/pages/api/bitbucket/refresh.ts (2 hunks)
  • apps/deploy-web/src/pages/api/github/authenticate.ts (2 hunks)
  • apps/deploy-web/src/pages/api/gitlab/authenticate.ts (2 hunks)
  • apps/deploy-web/src/pages/api/gitlab/refresh.ts (2 hunks)
  • apps/deploy-web/src/pages/api/proxy/[...path].ts (2 hunks)
  • apps/deploy-web/src/services/app-di-container/app-di-container.ts (1 hunks)
  • apps/deploy-web/src/services/error-handler/error-handler.service.spec.ts (3 hunks)
  • apps/deploy-web/src/services/error-handler/error-handler.service.ts (1 hunks)
  • apps/deploy-web/src/services/feature-flag/index.ts (0 hunks)
  • apps/deploy-web/src/services/http/http-server.service.ts (2 hunks)
💤 Files with no reviewable changes (1)
  • apps/deploy-web/src/services/feature-flag/index.ts
✅ Files skipped from review due to trivial changes (2)
  • apps/deploy-web/src/lib/nextjs/getValidatedServerSideProps.ts
  • apps/deploy-web/src/lib/nextjs/getServerSidePropsWithServices.ts
🚧 Files skipped from review as they are similar to previous changes (12)
  • apps/deploy-web/src/instrumentation.ts
  • apps/deploy-web/src/pages/api/bitbucket/refresh.ts
  • apps/deploy-web/src/pages/api/github/authenticate.ts
  • apps/deploy-web/src/pages/api/gitlab/authenticate.ts
  • apps/deploy-web/src/pages/api/auth/signup.ts
  • apps/deploy-web/src/services/app-di-container/app-di-container.ts
  • apps/deploy-web/src/pages/api/auth/[...auth0].ts
  • apps/deploy-web/src/middleware.ts
  • apps/deploy-web/src/pages/api/proxy/[...path].ts
  • apps/deploy-web/src/pages/api/gitlab/refresh.ts
  • apps/deploy-web/src/pages/api/bitbucket/authenticate.ts
  • apps/deploy-web/src/services/error-handler/error-handler.service.spec.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.spec.{ts,tsx}`: Use `setup` function instead of `beforeEach` in test files...

**/*.spec.{ts,tsx}: Use setup function instead of beforeEach in test files
setup function must be at the bottom of the root describe block
setup function creates an object under test and returns it
setup function should accept a single parameter with inline type definition
Don't use shared state in setup function
Don't specify return type of setup function

📄 Source: CodeRabbit Inference Engine (.cursor/rules/setup-instead-of-before-each.mdc)

List of files the instruction was applied to:

  • apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts
  • apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: jzsfkzm
PR: akash-network/console#1372
File: apps/api/src/dashboard/services/stats/stats.service.ts:0-0
Timestamp: 2025-05-28T20:42:58.200Z
Learning: The user successfully implemented CosmosHttpService with retry logic using axiosRetry, exponential backoff, and proper error handling for Cosmos API calls, replacing direct axios usage in StatsService.
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
Learnt from: jzsfkzm
PR: akash-network/console#1364
File: apps/api/src/provider/routes/provider-deployments/provider-deployments.router.ts:38-44
Timestamp: 2025-05-25T19:37:00.800Z
Learning: The Akash Console API uses centralized error handling through the OpenApiHonoHandler's defaultHook, which automatically processes errors via HonoErrorHandlerService. Individual route handlers do not use explicit try-catch blocks and instead rely on this framework-level error handling mechanism.
apps/deploy-web/next.config.js (2)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
apps/deploy-web/src/services/http/http-server.service.ts (1)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts (6)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts (7)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
apps/deploy-web/src/services/error-handler/error-handler.service.ts (2)
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
Learnt from: jzsfkzm
PR: akash-network/console#1364
File: apps/api/src/provider/routes/provider-deployments/provider-deployments.router.ts:38-44
Timestamp: 2025-05-25T19:37:00.800Z
Learning: The Akash Console API uses centralized error handling through the OpenApiHonoHandler's defaultHook, which automatically processes errors via HonoErrorHandlerService. Individual route handlers do not use explicit try-catch blocks and instead rely on this framework-level error handling mechanism.
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (1)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
🧬 Code Graph Analysis (5)
apps/deploy-web/src/services/http/http-server.service.ts (1)
apps/deploy-web/src/services/feature-flag/feature-flag.service.ts (1)
  • FeatureFlagService (7-48)
apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts (3)
apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.ts (3)
  • isRegisteredUser (11-14)
  • isFeatureEnabled (6-9)
  • redirectIfAccessTokenExpired (16-31)
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (1)
  • AppTypedContext (22-26)
apps/deploy-web/src/services/feature-flag/feature-flag.service.ts (1)
  • FeatureFlagService (7-48)
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 (22-26)
apps/deploy-web/src/services/error-handler/error-handler.service.ts (2)
packages/http-sdk/src/utils/isHttpError.ts (1)
  • isHttpError (3-5)
packages/logging/src/servicies/logger/logger.service.ts (1)
  • error (107-109)
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (2)
apps/api/src/core/services/execution-context/execution-context.service.ts (1)
  • context (18-26)
apps/deploy-web/src/services/http/http-server.service.ts (1)
  • services (21-25)
⏰ 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)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
🔇 Additional comments (21)
apps/deploy-web/next.config.js (1)

26-26: LGTM! Necessary addition for test environment compatibility.

The addition of "@panva/hkdf" and "jose" to the transpilePackages array for the test environment is appropriate. These are legitimate cryptographic and JWT libraries that commonly require transpilation in test environments due to ES module compatibility issues.

apps/deploy-web/src/services/http/http-server.service.ts (3)

2-2: LGTM! Import pattern improvement.

The wildcard import of the unleash module provides better encapsulation and aligns with the dependency injection pattern used in the service constructor.


9-9: LGTM! Dependency injection pattern.

Good refactoring from singleton import to class import, enabling proper dependency injection and improving testability.


23-23: LGTM! Per-instance service creation.

The change from singleton to per-instance creation with explicit dependency injection is a solid architectural improvement. This enhances modularity and makes the service more testable.

apps/deploy-web/src/services/error-handler/error-handler.service.ts (5)

7-10: LGTM! Enhanced constructor with dependency injection.

The constructor now properly injects the LoggerService dependency, improving testability and following good dependency injection practices.


12-18: LGTM! Sentry trace data integration.

The getTraceData() method provides structured access to Sentry trace information, which will improve debugging and error correlation.


20-36: LGTM! Structured error reporting.

The reportError method is a significant improvement over the previous handleError. It provides:

  • Structured error context with tags and metadata
  • HTTP error enrichment with status, method, and URL
  • Integrated logging and Sentry reporting
  • Consistent error handling across the application

38-54: LGTM! Robust callback wrapping.

The wrapCallback method provides excellent error handling for both synchronous and asynchronous callbacks:

  • Handles both sync and async execution paths
  • Proper error reporting with optional tags
  • Fallback value support for graceful degradation
  • Type-safe generic implementation

57-74: LGTM! Well-designed TypeScript interfaces.

The new type definitions provide excellent structure:

  • SeverityLevel alias for Sentry severity levels
  • ErrorContext with extensible properties
  • WrapCallbackOptions with proper generics
  • TraceData for Sentry trace information
apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts (2)

10-94: LGTM! Comprehensive test coverage.

The test suite provides excellent coverage for all page guard functions:

  • isRegisteredUser: Tests both logged-in and not-logged-in scenarios
  • isFeatureEnabled: Tests feature flag evaluation and user ID passing
  • redirectIfAccessTokenExpired: Tests both valid and expired token scenarios

The test structure is clear and assertions are appropriate.


96-106: LGTM! Proper setup function implementation.

The setup function correctly follows all coding guidelines:

  • Uses inline type definition for parameter
  • Creates and returns object under test
  • No shared state or return type specification
  • Positioned at bottom of root describe block
  • Provides flexible mocking for different test scenarios
apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.ts (3)

6-9: LGTM! Feature flag check with user context.

The isFeatureEnabled function correctly retrieves the session and passes the user ID to the feature flag service for proper personalization.


11-14: LGTM! Simple and effective user registration check.

The isRegisteredUser function provides a clean way to check if a user is authenticated by verifying the session user object.


16-31: LGTM! Comprehensive access token expiry handling.

The redirectIfAccessTokenExpired function provides robust handling:

  • Proper timestamp conversion and comparison
  • Informative warning logging with request URL
  • Correct Next.js redirect structure
  • Preserves original URL for post-login redirect
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts (2)

14-213: LGTM! Comprehensive test coverage for defineServerSideProps.

The test suite provides excellent coverage for all scenarios:

  • Basic functionality with empty props
  • Handler execution and result passing
  • Schema validation with transformation
  • Error handling for validation failures
  • Conditional logic with boolean, redirect, and notFound results
  • Async/sync handler and condition support
  • Request context exposure
  • Edge cases with undefined/null conditions

The test structure is well-organized and assertions are thorough.


215-241: LGTM! Proper setup function implementation.

The setup function correctly follows all coding guidelines:

  • Uses inline type definition for parameter
  • Creates comprehensive mock context
  • Returns object under test
  • No shared state or return type specification
  • Positioned at bottom of root describe block
  • Provides flexible configuration for different test scenarios
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (5)

1-8: LGTM: Well-organized imports with proper integrations.

The imports are clean and demonstrate good integration with the established patterns - Sentry for error monitoring, proper Next.js types, and service injection from the centralized http-server service.


9-20: LGTM: Well-designed interface with excellent type safety.

The WrapServerSideOptions interface is well-structured with:

  • Clear documentation for the route parameter
  • Proper generic constraints for type safety
  • Flexible optional parameters for different use cases
  • Good separation of concerns between validation, conditional logic, and handler execution

22-33: LGTM: Sophisticated type definitions with excellent conditional typing.

The type definitions provide excellent type safety through:

  • Proper generic constraints and conditional types
  • Smart inference of params and query types from Zod schemas
  • Clean separation between typed and untyped contexts
  • Service injection integration through intersection types

35-37: LGTM: Clean constant definition.

Good practice to define reusable constants for common response patterns.


39-65: Excellent integration with established patterns.

The function demonstrates excellent alignment with the established patterns mentioned in the learnings:

  • Uses centralized error handling through Sentry (consistent with Akash Console's error handling approach)
  • Implements dependency injection pattern (aligning with the service instantiation approach)
  • Provides proper request-scoped execution context

The overall architecture is well-designed and follows best practices for Next.js server-side props handling.

@stalniy stalniy force-pushed the feat/observability-deploy-web-ssr branch 2 times, most recently from 7e8ab83 to 1183b0d Compare July 7, 2025 16:26
Copy link
Contributor

@baktun14 baktun14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@stalniy stalniy force-pushed the feat/observability-deploy-web-ssr branch from 1183b0d to f1c1df2 Compare July 8, 2025 16:33
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts (1)

164-176: Consider improving type safety in the headers test.

The test logic is correct, but the type assertion (result.props as any).headers.entries() bypasses TypeScript's type checking. Consider using a more specific type assertion if possible.

-    expect(Object.fromEntries((result.props as any).headers.entries())).toEqual(headers);
+    expect(Object.fromEntries((result.props.headers as Headers).entries())).toEqual(headers);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1183b0d and f1c1df2.

📒 Files selected for processing (24)
  • apps/deploy-web/next.config.js (1 hunks)
  • apps/deploy-web/src/instrumentation.ts (2 hunks)
  • apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts (1 hunks)
  • apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts (1 hunks)
  • apps/deploy-web/src/lib/nextjs/getServerSidePropsWithServices.ts (1 hunks)
  • apps/deploy-web/src/lib/nextjs/getValidatedServerSideProps.ts (1 hunks)
  • apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts (1 hunks)
  • apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.ts (1 hunks)
  • apps/deploy-web/src/middleware.ts (2 hunks)
  • apps/deploy-web/src/pages/alerts/notification-channels/[id]/index.tsx (3 hunks)
  • apps/deploy-web/src/pages/api/auth/[...auth0].ts (3 hunks)
  • apps/deploy-web/src/pages/api/auth/signup.ts (2 hunks)
  • apps/deploy-web/src/pages/api/bitbucket/authenticate.ts (2 hunks)
  • apps/deploy-web/src/pages/api/bitbucket/refresh.ts (2 hunks)
  • apps/deploy-web/src/pages/api/github/authenticate.ts (2 hunks)
  • apps/deploy-web/src/pages/api/gitlab/authenticate.ts (2 hunks)
  • apps/deploy-web/src/pages/api/gitlab/refresh.ts (2 hunks)
  • apps/deploy-web/src/pages/api/proxy/[...path].ts (2 hunks)
  • apps/deploy-web/src/services/app-di-container/app-di-container.ts (1 hunks)
  • apps/deploy-web/src/services/error-handler/error-handler.service.spec.ts (3 hunks)
  • apps/deploy-web/src/services/error-handler/error-handler.service.ts (1 hunks)
  • apps/deploy-web/src/services/feature-flag/index.ts (0 hunks)
  • apps/deploy-web/src/services/http/http-server.service.ts (2 hunks)
  • apps/deploy-web/src/services/route-protector/index.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • apps/deploy-web/src/services/feature-flag/index.ts
🚧 Files skipped from review as they are similar to previous changes (20)
  • apps/deploy-web/src/lib/nextjs/getServerSidePropsWithServices.ts
  • apps/deploy-web/src/services/app-di-container/app-di-container.ts
  • apps/deploy-web/next.config.js
  • apps/deploy-web/src/pages/api/github/authenticate.ts
  • apps/deploy-web/src/pages/api/bitbucket/authenticate.ts
  • apps/deploy-web/src/pages/api/auth/signup.ts
  • apps/deploy-web/src/lib/nextjs/getValidatedServerSideProps.ts
  • apps/deploy-web/src/middleware.ts
  • apps/deploy-web/src/services/route-protector/index.ts
  • apps/deploy-web/src/instrumentation.ts
  • apps/deploy-web/src/pages/api/gitlab/refresh.ts
  • apps/deploy-web/src/pages/api/bitbucket/refresh.ts
  • apps/deploy-web/src/pages/api/proxy/[...path].ts
  • apps/deploy-web/src/services/http/http-server.service.ts
  • apps/deploy-web/src/pages/api/auth/[...auth0].ts
  • apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.spec.ts
  • apps/deploy-web/src/pages/api/gitlab/authenticate.ts
  • apps/deploy-web/src/pages/alerts/notification-channels/[id]/index.tsx
  • apps/deploy-web/src/lib/nextjs/pageGuards/pageGuards.ts
  • apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.spec.{ts,tsx}`: Use `setup` function instead of `beforeEach` in test files...

**/*.spec.{ts,tsx}: Use setup function instead of beforeEach in test files
setup function must be at the bottom of the root describe block
setup function creates an object under test and returns it
setup function should accept a single parameter with inline type definition
Don't use shared state in setup function
Don't specify return type of setup function

📄 Source: CodeRabbit Inference Engine (.cursor/rules/setup-instead-of-before-each.mdc)

List of files the instruction was applied to:

  • apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts
  • apps/deploy-web/src/services/error-handler/error-handler.service.spec.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: jzsfkzm
PR: akash-network/console#1372
File: apps/api/src/dashboard/services/stats/stats.service.ts:0-0
Timestamp: 2025-05-28T20:42:58.200Z
Learning: The user successfully implemented CosmosHttpService with retry logic using axiosRetry, exponential backoff, and proper error handling for Cosmos API calls, replacing direct axios usage in StatsService.
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
Learnt from: jzsfkzm
PR: akash-network/console#1364
File: apps/api/src/provider/routes/provider-deployments/provider-deployments.router.ts:38-44
Timestamp: 2025-05-25T19:37:00.800Z
Learning: The Akash Console API uses centralized error handling through the OpenApiHonoHandler's defaultHook, which automatically processes errors via HonoErrorHandlerService. Individual route handlers do not use explicit try-catch blocks and instead rely on this framework-level error handling mechanism.
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts (7)
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-06-30T12:11:50.570Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use shared state in `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
apps/deploy-web/src/services/error-handler/error-handler.service.spec.ts (6)
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function creates an object under test and returns it
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function should accept a single parameter with inline type definition
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-30T12:11:58.081Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't specify return type of `setup` function
apps/deploy-web/src/services/error-handler/error-handler.service.ts (2)
Learnt from: jzsfkzm
PR: akash-network/console#1498
File: apps/api/src/services/external/templates/template-fetcher.service.ts:0-0
Timestamp: 2025-07-03T14:40:49.886Z
Learning: In the TemplateFetcherService class in apps/api/src/services/external/templates/template-fetcher.service.ts, the error handling strategy should maintain process resilience by catching all errors and returning null rather than re-throwing critical errors, to avoid breaking the whole template fetching process.
Learnt from: jzsfkzm
PR: akash-network/console#1364
File: apps/api/src/provider/routes/provider-deployments/provider-deployments.router.ts:38-44
Timestamp: 2025-05-25T19:37:00.800Z
Learning: The Akash Console API uses centralized error handling through the OpenApiHonoHandler's defaultHook, which automatically processes errors via HonoErrorHandlerService. Individual route handlers do not use explicit try-catch blocks and instead rely on this framework-level error handling mechanism.
⏰ 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)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
🔇 Additional comments (23)
apps/deploy-web/src/lib/nextjs/defineServerSideProps/defineServerSideProps.spec.ts (14)

1-13: LGTM! Clean imports and mocking setup.

The imports are well-organized and the server environment config mock is appropriate for testing.


15-19: LGTM! Test correctly validates default behavior.

The test properly verifies that when no configuration is provided, the function returns empty props.


21-35: LGTM! Handler execution test is comprehensive.

The test correctly verifies that the handler is called with the expected context including services, and the result is properly returned.


37-66: LGTM! Schema validation test demonstrates proper transformation.

The test effectively validates that Zod schema validation works correctly, including data transformation (string to number conversion) and proper context passing to the handler.


68-84: LGTM! Error handling test is appropriate.

The test correctly verifies that schema validation failures throw appropriate errors with the expected error message pattern.


86-100: LGTM! Conditional guard tests cover sync and async scenarios.

The tests properly verify that both synchronous and asynchronous if conditions returning false result in notFound: true.


102-117: LGTM! Redirect handling test is well-structured.

The test correctly validates that redirect results from if conditions are properly passed through without modification.


119-131: LGTM! Conditional execution flow is properly tested.

The test validates that when the if condition returns true, the handler is executed and its result is returned.


133-145: LGTM! Async conditional execution is covered.

The test ensures that Promise<true> from if conditions properly triggers handler execution.


147-162: LGTM! Async condition result handling is comprehensive.

The test validates that async if conditions can return GetServerSidePropsResult objects that are properly passed through.


178-192: LGTM! Async and sync handler support is properly tested.

The tests correctly verify that both synchronous and asynchronous handlers are supported and work as expected.


194-213: LGTM! Edge case handling for null/undefined conditions is thorough.

The test properly validates that undefined and null returns from if conditions are treated as truthy, allowing handler execution.


215-241: LGTM! Setup function follows coding guidelines perfectly.

The setup function is correctly positioned at the bottom of the root describe block, accepts a single parameter with inline type definition, doesn't use shared state, has no return type specified, and creates the object under test properly.


243-244: LGTM! Type definition is appropriate.

The Request type extension properly adds the optional originalUrl property to the Next.js request type.

apps/deploy-web/src/services/error-handler/error-handler.service.ts (5)

7-10: LGTM! Good dependency injection pattern.

The constructor properly follows dependency injection principles by accepting the LoggerService instance and making captureException injectable with a default value. This improves testability and maintainability.


12-18: LGTM! Clean trace data extraction.

The getTraceData method provides a clean interface for retrieving Sentry trace identifiers, which will improve observability and debugging capabilities.


57-74: LGTM! Well-structured TypeScript interfaces.

The type definitions provide excellent type safety and extensibility. The ErrorContext interface with index signature allows for flexible error metadata while maintaining type safety for core properties.


20-36: Verify 400-status exclusion is intentional

I searched the entire codebase and this status !== 400 filter only appears in:

  • File: apps/deploy-web/src/services/error-handler/error-handler.service.ts
    Lines: 20–36

If the intent is to skip enriching Sentry/log tags for HTTP 400 errors (e.g. to avoid logging client-side validation failures), please confirm. Otherwise, consider removing this special case or documenting why 400s are excluded.


38-54: No changes needed for promise detection logic in wrapCallback.

Our search confirmed this is the only instance of using typeof result.catch === "function". The current detection correctly identifies Promise-like values without introducing unintended return-type changes. Adding an extra typeof result === "object" check is redundant, and wrapping everything in Promise.resolve() would force synchronous callbacks to return a Promise, altering the function’s behavior.

Likely an incorrect or invalid review comment.

apps/deploy-web/src/services/error-handler/error-handler.service.spec.ts (4)

9-22: LGTM! Test properly updated for new reportError method.

The test correctly verifies that both the logger and captureException are called with the expected parameters. The error context structure is properly validated.


24-64: LGTM! Comprehensive HTTP error handling test.

The test thoroughly covers HTTP error scenarios with proper mock setup and validates that HTTP-specific tags are correctly extracted and applied.


66-183: LGTM! Excellent test coverage for wrapCallback method.

The test suite comprehensively covers the wrapCallback functionality:

  • Synchronous and asynchronous error handling
  • Fallback value behavior for both sync and async functions
  • Success scenarios where no errors occur
  • Proper error reporting with tags

The test structure is well-organized and follows testing best practices.


185-189: LGTM! Setup function follows coding guidelines correctly.

The setup function is properly positioned at the bottom of the describe block, accepts a single parameter with inline type definition, doesn't specify a return type, and creates the object under test appropriately.

@stalniy stalniy merged commit ad55727 into main Jul 8, 2025
57 checks passed
@stalniy stalniy deleted the feat/observability-deploy-web-ssr branch July 8, 2025 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments