-
Notifications
You must be signed in to change notification settings - Fork 618
fix(billing): route subscription/sign-in/credit preconditions to modal, out of error panel (FE-878) #12785
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
dante01yoon
merged 4 commits into
main
from
jaewon/fe-878-bug-subscription-required-error-message-feels-choppyabrupt
Jun 19, 2026
Merged
fix(billing): route subscription/sign-in/credit preconditions to modal, out of error panel (FE-878) #12785
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
4d414ca
fix(billing): route subscription/sign-in/credit preconditions to moda…
dante01yoon dc6c48c
fix(billing): route queue paywall (402) to modal, out of error panel …
dante01yoon f8993d2
Merge branch 'main' into jaewon/fe-878-bug-subscription-required-erro…
dante01yoon 75131c2
refactor(billing): drop unused precedence helper, reuse RuntimeErrorI…
dante01yoon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| import type { Page } from '@playwright/test' | ||
| import { expect } from '@playwright/test' | ||
|
|
||
| import type { PromptResponse } from '@/schemas/apiSchema' | ||
|
|
||
| import { comfyPageFixture as test } from '@e2e/fixtures/ComfyPage' | ||
| import { TestIds } from '@e2e/fixtures/selectors' | ||
|
|
||
| // Regression for #12840: a free-tier paywall on queue (`POST /prompt` 402 with | ||
| // `{ error: { type: 'PAYMENT_REQUIRED', message: 'Subscription required to | ||
| // queue workflows' } }`) is an account precondition. It must open its own modal | ||
| // and stay out of the error panel, instead of surfacing the raw backend string | ||
| // with non-actionable Find-on-GitHub / Copy actions. | ||
| test.describe('Subscription paywall on queue', { tag: '@ui' }, () => { | ||
| test.beforeEach(async ({ comfyPage }) => { | ||
| await comfyPage.settings.setSetting( | ||
| 'Comfy.RightSidePanel.ShowErrorsTab', | ||
| true | ||
| ) | ||
| }) | ||
|
|
||
| async function mockQueueError(page: Page, error: PromptResponse['error']) { | ||
| const body: PromptResponse = { node_errors: {}, error } | ||
| await page.route('**/api/prompt', async (route) => { | ||
| await route.fulfill({ | ||
| status: 402, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify(body) | ||
| }) | ||
| }) | ||
| } | ||
|
|
||
| test('keeps the subscription paywall out of the error panel', async ({ | ||
| comfyPage | ||
| }) => { | ||
| await mockQueueError(comfyPage.page, { | ||
| type: 'PAYMENT_REQUIRED', | ||
| message: 'Subscription required to queue workflows', | ||
| details: '' | ||
| }) | ||
|
|
||
| const queued = comfyPage.page.waitForResponse('**/api/prompt') | ||
| await comfyPage.actionbar.queueButton.primaryButton.click() | ||
| await queued | ||
|
|
||
| await expect( | ||
| comfyPage.page.getByTestId(TestIds.dialogs.errorOverlay) | ||
| ).toBeHidden() | ||
| }) | ||
|
|
||
| test('still surfaces ordinary queue errors in the error panel', async ({ | ||
| comfyPage | ||
| }) => { | ||
| await mockQueueError(comfyPage.page, { | ||
| type: 'server_error', | ||
| message: 'The server exploded', | ||
| details: '' | ||
| }) | ||
|
|
||
| const queued = comfyPage.page.waitForResponse('**/api/prompt') | ||
| await comfyPage.actionbar.queueButton.primaryButton.click() | ||
| await queued | ||
|
|
||
| await expect( | ||
| comfyPage.page.getByTestId(TestIds.dialogs.errorOverlay) | ||
| ).toBeVisible() | ||
| }) | ||
| }) |
58 changes: 58 additions & 0 deletions
58
src/platform/cloud/subscription/composables/useAccountPreconditionDialog.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest' | ||
|
|
||
| import { useAccountPreconditionDialog } from './useAccountPreconditionDialog' | ||
|
|
||
| const mockDialogService = { | ||
| showApiNodesSignInDialog: vi.fn(), | ||
| showSubscriptionRequiredDialog: vi.fn(), | ||
| showTopUpCreditsDialog: vi.fn() | ||
| } | ||
|
|
||
| vi.mock('@/services/dialogService', () => ({ | ||
| useDialogService: vi.fn(() => mockDialogService) | ||
| })) | ||
|
|
||
| describe('useAccountPreconditionDialog', () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks() | ||
| }) | ||
|
|
||
| it('routes a sign-in precondition to the API sign-in dialog with the node type', () => { | ||
| useAccountPreconditionDialog().open('sign_in', { nodeType: 'ApiNode' }) | ||
|
|
||
| expect(mockDialogService.showApiNodesSignInDialog).toHaveBeenCalledWith([ | ||
| 'ApiNode' | ||
| ]) | ||
| expect( | ||
| mockDialogService.showSubscriptionRequiredDialog | ||
| ).not.toHaveBeenCalled() | ||
| expect(mockDialogService.showTopUpCreditsDialog).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it('routes a sign-in precondition with no node type to an empty list', () => { | ||
| useAccountPreconditionDialog().open('sign_in') | ||
|
|
||
| expect(mockDialogService.showApiNodesSignInDialog).toHaveBeenCalledWith([]) | ||
| }) | ||
|
|
||
| it('routes a subscription precondition to the subscription dialog', () => { | ||
| useAccountPreconditionDialog().open('subscription') | ||
|
|
||
| expect( | ||
| mockDialogService.showSubscriptionRequiredDialog | ||
| ).toHaveBeenCalledTimes(1) | ||
| expect(mockDialogService.showApiNodesSignInDialog).not.toHaveBeenCalled() | ||
| expect(mockDialogService.showTopUpCreditsDialog).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it('routes a credit precondition to the top-up dialog', () => { | ||
| useAccountPreconditionDialog().open('credits', { nodeType: 'PartnerNode' }) | ||
|
|
||
| expect(mockDialogService.showTopUpCreditsDialog).toHaveBeenCalledWith({ | ||
| isInsufficientCredits: true | ||
| }) | ||
| expect( | ||
| mockDialogService.showSubscriptionRequiredDialog | ||
| ).not.toHaveBeenCalled() | ||
| }) | ||
| }) |
38 changes: 38 additions & 0 deletions
38
src/platform/cloud/subscription/composables/useAccountPreconditionDialog.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import type { AccountPrecondition } from '@/platform/errorCatalog/accountPreconditionRouting' | ||
| import { useDialogService } from '@/services/dialogService' | ||
|
|
||
| export interface AccountPreconditionContext { | ||
| /** Node type that triggered the precondition, used as modal context. */ | ||
| nodeType?: string | ||
| } | ||
|
|
||
| // Routes a resolved account precondition to its dedicated modal. This is the | ||
| // single seam where FE-978 attaches role-aware (member vs owner) subscription | ||
| // content: the `subscription` branch resolves to the subscription dialog, whose | ||
| // inner content FE-978 specializes for cancelled/inactive team states. | ||
| export function useAccountPreconditionDialog() { | ||
| const dialogService = useDialogService() | ||
|
|
||
| function open( | ||
| precondition: AccountPrecondition, | ||
| context: AccountPreconditionContext = {} | ||
| ): void { | ||
| switch (precondition) { | ||
| case 'sign_in': | ||
| void dialogService.showApiNodesSignInDialog( | ||
| context.nodeType ? [context.nodeType] : [] | ||
| ) | ||
| return | ||
| case 'subscription': | ||
| void dialogService.showSubscriptionRequiredDialog() | ||
| return | ||
| case 'credits': | ||
| void dialogService.showTopUpCreditsDialog({ | ||
| isInsufficientCredits: true | ||
| }) | ||
| return | ||
| } | ||
| } | ||
|
|
||
| return { open } | ||
| } |
141 changes: 141 additions & 0 deletions
141
src/platform/errorCatalog/accountPreconditionRouting.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,141 @@ | ||
| import { describe, expect, it } from 'vitest' | ||
|
|
||
| import { | ||
| isAccountPreconditionCatalogId, | ||
| preconditionForCatalogId, | ||
| resolveAccountPrecondition, | ||
| selectHighestPrecedencePrecondition | ||
| } from './accountPreconditionRouting' | ||
| import { | ||
| EXECUTION_FAILED_CATALOG_ID, | ||
| INSUFFICIENT_CREDITS_CATALOG_ID, | ||
| SIGN_IN_REQUIRED_CATALOG_ID, | ||
| SUBSCRIPTION_REQUIRED_CATALOG_ID, | ||
| SUBSCRIPTION_UPGRADE_REQUIRED_CATALOG_ID, | ||
| WORKSPACE_INSUFFICIENT_CREDITS_CATALOG_ID | ||
| } from './catalogIds' | ||
|
|
||
| describe('resolveAccountPrecondition', () => { | ||
| it('classifies a sign-in error', () => { | ||
| expect( | ||
| resolveAccountPrecondition({ | ||
| exceptionType: 'RuntimeError', | ||
| exceptionMessage: 'Unauthorized: Please login first to use this node.' | ||
| }) | ||
| ).toBe('sign_in') | ||
| }) | ||
|
|
||
| it('classifies an inactive-subscription error', () => { | ||
| expect( | ||
| resolveAccountPrecondition({ | ||
| exceptionType: 'InactiveSubscriptionError', | ||
| exceptionMessage: | ||
| 'User has no active subscription. Please subscribe to a plan to continue.' | ||
| }) | ||
| ).toBe('subscription') | ||
| }) | ||
|
|
||
| it('classifies the queue paywall (PAYMENT_REQUIRED) as a subscription precondition', () => { | ||
| expect( | ||
| resolveAccountPrecondition({ | ||
| exceptionType: 'PAYMENT_REQUIRED', | ||
| exceptionMessage: 'Subscription required to queue workflows' | ||
| }) | ||
| ).toBe('subscription') | ||
| }) | ||
|
|
||
| it('classifies a subscription-upgrade error as a subscription precondition', () => { | ||
| expect( | ||
| resolveAccountPrecondition({ | ||
| exceptionType: 'RuntimeError', | ||
| exceptionMessage: | ||
| 'the following private models require a subscription upgrade: flux-pro' | ||
| }) | ||
| ).toBe('subscription') | ||
| }) | ||
|
|
||
| it('classifies an account credit error', () => { | ||
| expect( | ||
| resolveAccountPrecondition({ | ||
| exceptionType: 'InsufficientFundsError', | ||
| exceptionMessage: | ||
| 'Payment Required: Please add credits to your account to use this node.' | ||
| }) | ||
| ).toBe('credits') | ||
| }) | ||
|
|
||
| it('classifies a workspace credit error', () => { | ||
| expect( | ||
| resolveAccountPrecondition({ | ||
| exceptionType: 'RuntimeError', | ||
| exceptionMessage: | ||
| 'Payment Required: Please add credits to your workspace to continue.' | ||
| }) | ||
| ).toBe('credits') | ||
| }) | ||
|
|
||
| it('returns undefined for an ordinary workflow error', () => { | ||
| expect( | ||
| resolveAccountPrecondition({ | ||
| exceptionType: 'RuntimeError', | ||
| exceptionMessage: 'CUDA out of memory' | ||
| }) | ||
| ).toBeUndefined() | ||
| }) | ||
| }) | ||
|
|
||
| describe('preconditionForCatalogId / isAccountPreconditionCatalogId', () => { | ||
| it('maps every precondition catalog id', () => { | ||
| expect(preconditionForCatalogId(SIGN_IN_REQUIRED_CATALOG_ID)).toBe( | ||
| 'sign_in' | ||
| ) | ||
| expect(preconditionForCatalogId(SUBSCRIPTION_REQUIRED_CATALOG_ID)).toBe( | ||
| 'subscription' | ||
| ) | ||
| expect( | ||
| preconditionForCatalogId(SUBSCRIPTION_UPGRADE_REQUIRED_CATALOG_ID) | ||
| ).toBe('subscription') | ||
| expect(preconditionForCatalogId(INSUFFICIENT_CREDITS_CATALOG_ID)).toBe( | ||
| 'credits' | ||
| ) | ||
| expect( | ||
| preconditionForCatalogId(WORKSPACE_INSUFFICIENT_CREDITS_CATALOG_ID) | ||
| ).toBe('credits') | ||
| }) | ||
|
|
||
| it('does not treat a workflow error catalog id as a precondition', () => { | ||
| expect( | ||
| preconditionForCatalogId(EXECUTION_FAILED_CATALOG_ID) | ||
| ).toBeUndefined() | ||
| expect(isAccountPreconditionCatalogId(EXECUTION_FAILED_CATALOG_ID)).toBe( | ||
| false | ||
| ) | ||
| expect(isAccountPreconditionCatalogId(undefined)).toBe(false) | ||
| }) | ||
| }) | ||
|
|
||
| describe('selectHighestPrecedencePrecondition', () => { | ||
| it('prefers sign-in over subscription and credits', () => { | ||
| expect( | ||
| selectHighestPrecedencePrecondition([ | ||
| 'credits', | ||
| 'subscription', | ||
| 'sign_in' | ||
| ]) | ||
| ).toBe('sign_in') | ||
| }) | ||
|
|
||
| it('prefers subscription over credits', () => { | ||
| expect( | ||
| selectHighestPrecedencePrecondition(['credits', 'subscription']) | ||
| ).toBe('subscription') | ||
| }) | ||
|
|
||
| it('returns the only precondition present', () => { | ||
| expect(selectHighestPrecedencePrecondition(['credits'])).toBe('credits') | ||
| }) | ||
|
|
||
| it('returns undefined when none are present', () => { | ||
| expect(selectHighestPrecedencePrecondition([])).toBeUndefined() | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| import { | ||
| INSUFFICIENT_CREDITS_CATALOG_ID, | ||
| SIGN_IN_REQUIRED_CATALOG_ID, | ||
| SUBSCRIPTION_REQUIRED_CATALOG_ID, | ||
| SUBSCRIPTION_UPGRADE_REQUIRED_CATALOG_ID, | ||
| WORKSPACE_INSUFFICIENT_CREDITS_CATALOG_ID | ||
| } from './catalogIds' | ||
| import { resolveRuntimeCatalogMatch } from './runtimeErrorMatcher' | ||
|
|
||
| // Account preconditions are gating states (sign-in, subscription, credits) that | ||
| // must open their own modal instead of surfacing as a workflow error. They are | ||
| // excluded from the error panel and the error count. | ||
| export type AccountPrecondition = 'sign_in' | 'subscription' | 'credits' | ||
|
|
||
| interface AccountPreconditionInfo { | ||
| exceptionType: string | ||
| exceptionMessage: string | ||
| } | ||
|
|
||
| // Lower index wins. Sign-in must resolve before a subscription prompt, and any | ||
| // payment precondition takes precedence over a credits top-up. | ||
| const PRECONDITION_PRECEDENCE: AccountPrecondition[] = [ | ||
| 'sign_in', | ||
| 'subscription', | ||
| 'credits' | ||
| ] | ||
|
|
||
| const CATALOG_ID_TO_PRECONDITION = new Map<string, AccountPrecondition>([ | ||
| [SIGN_IN_REQUIRED_CATALOG_ID, 'sign_in'], | ||
| [SUBSCRIPTION_REQUIRED_CATALOG_ID, 'subscription'], | ||
| [SUBSCRIPTION_UPGRADE_REQUIRED_CATALOG_ID, 'subscription'], | ||
| [INSUFFICIENT_CREDITS_CATALOG_ID, 'credits'], | ||
| [WORKSPACE_INSUFFICIENT_CREDITS_CATALOG_ID, 'credits'] | ||
| ]) | ||
|
|
||
| export function preconditionForCatalogId( | ||
| catalogId: string | undefined | ||
| ): AccountPrecondition | undefined { | ||
| if (!catalogId) return undefined | ||
| return CATALOG_ID_TO_PRECONDITION.get(catalogId) | ||
| } | ||
|
|
||
| export function isAccountPreconditionCatalogId( | ||
| catalogId: string | undefined | ||
| ): boolean { | ||
| return preconditionForCatalogId(catalogId) !== undefined | ||
| } | ||
|
|
||
| // Classifies a single runtime error payload into the account precondition it | ||
| // represents, or `undefined` when it is an ordinary workflow error. | ||
| export function resolveAccountPrecondition( | ||
| info: AccountPreconditionInfo | ||
| ): AccountPrecondition | undefined { | ||
| const match = resolveRuntimeCatalogMatch(info) | ||
| return preconditionForCatalogId(match?.catalogId) | ||
| } | ||
|
|
||
| // Resolves the winning precondition when several could co-occur. Ordering | ||
| // follows sign-in -> subscription -> credits. | ||
| export function selectHighestPrecedencePrecondition( | ||
| preconditions: Iterable<AccountPrecondition> | ||
| ): AccountPrecondition | undefined { | ||
| const present = new Set(preconditions) | ||
| return PRECONDITION_PRECEDENCE.find((candidate) => present.has(candidate)) | ||
| } | ||
|
dante01yoon marked this conversation as resolved.
Outdated
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.