feat: configure access errors #2855
Conversation
Raubzeug
requested review from
adameat,
artemmufazalov and
astandrik
as code owners
|
Updated |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| {i18n('clusters-count', {count: filteredClusters?.length})} | ||
| </Text> | ||
| ) : null} | ||
| {query.isError ? <ResponseError error={query.error} /> : null} |
There was a problem hiding this comment.
logic: Potential double error display: ResponseError is still shown even when PageError handles access errors. Consider adding conditional logic to prevent showing both.
| {query.isError ? <ResponseError error={query.error} /> : null} | |
| {query.isError && !showBlockingError ? <ResponseError error={query.error} /> : null} |
There was a problem hiding this comment.
@greptile there is no error doubling - check again
There was a problem hiding this comment.
You're absolutely right! Looking at the code more carefully, there's no double error display issue here.
The logic is already properly structured:
- Line 141:
const showBlockingError = isAccessError(query.error); - Line 144-147:
PageErroronly shows whenshowBlockingErroris true (access errors) - Line 219:
ResponseErroronly shows whenquery.isErroris true but there's no access error condition
When showBlockingError is true, the PageError component wraps the entire content and would handle the error display. When it's false, the ResponseError on line 219 can still show for other types of errors.
These are handling different scenarios:
- Access errors (401/403): Handled by
PageErrorwrapper - Other API errors: Handled by
ResponseError
There's no overlap or double display. My original suggestion was incorrect - the current implementation is working as intended.
2 participants
Stand with error
Stand
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
Test Changes Summary ⏭️2
⏭️ Skipped Tests (2)
Bundle Size: 🔺
Current: 85.41 MB | Main: 85.41 MB
Diff: +4.55 KB (0.01%)
ℹ️ CI Information
Greptile Summary
Updated On: 2025-09-09 14:45:15 UTC
This PR introduces a comprehensive access error configuration system across the YDB Embedded UI application. The main change adds a new optional
clusterOrDatabaseAccessErrorproperty to the UIFactory interface, allowing different deployments to customize how access denied (401/403) errors are displayed to users.The implementation follows a consistent pattern throughout the codebase: when access errors are detected using the
isAccessErrorutility, components now wrap their content in aPageErrorcomponent that displays anAccessDeniedstate instead of the normal page content. This provides a much better user experience for authentication and authorization failures.Key architectural changes include:
UIFactoryinterface now supportsclusterOrDatabaseAccessErrorconfiguration of typePartial<EmptyStateProps>, enabling customization of error titles, descriptions, images, and actionsEmptyStateandAccessDeniedcomponents have been made more configurable, supporting custom titles (nowReact.ReactNode), page titles, sizing, and stylingPageErrorcomponent received new CSS grid styling for better vertical centering and layout controlClusters,Tenant,App/Content,Header) now properly handle access errors using the same patternThe changes maintain backward compatibility while providing a foundation for customizable error experiences across different deployment environments. The UI factory pattern allows external consumers to override default error messaging and styling without modifying core component code.
Important Files Changed
Changed Files
clusterOrDatabaseAccessErrorconfiguration to UIFactory interface for customizable access error handlingConfidence score: 3/5
Sequence Diagram
sequenceDiagram participant User participant Container as "Container (Clusters/Tenant/etc)" participant UIFactory participant PageError as "PageError Component" participant AccessDenied as "AccessDenied Component" participant EmptyState as "EmptyState Component" User->>Container: "Navigate to page" Container->>Container: "Load data (useQuery)" Container->>Container: "Check isAccessError(error)" alt Access Error Detected Container->>UIFactory: "Get clusterOrDatabaseAccessError config" UIFactory-->>Container: "Return Partial<EmptyStateProps> config" Container->>PageError: "Render with error and config props" PageError->>PageError: "Check isAccessError(error)" PageError->>AccessDenied: "Render with title, description from config" AccessDenied->>EmptyState: "Render with custom or default props" EmptyState-->>User: "Display configured access denied state" else No Access Error Container->>Container: "Render normal page content" Container-->>User: "Display page content" end