feat: handle new command errors better (#1318)
This commit is contained in:
@@ -16,6 +16,7 @@ export interface AuthCredentials {
|
||||
export interface UserContext {
|
||||
orgId?: string;
|
||||
orgName?: string;
|
||||
orgSlug?: string;
|
||||
briefId?: string;
|
||||
briefName?: string;
|
||||
updatedAt: string;
|
||||
|
||||
@@ -52,7 +52,10 @@ export const ERROR_CODES = {
|
||||
INVALID_INPUT: 'INVALID_INPUT',
|
||||
NOT_IMPLEMENTED: 'NOT_IMPLEMENTED',
|
||||
UNKNOWN_ERROR: 'UNKNOWN_ERROR',
|
||||
NOT_FOUND: 'NOT_FOUND'
|
||||
NOT_FOUND: 'NOT_FOUND',
|
||||
|
||||
// Context errors
|
||||
NO_BRIEF_SELECTED: 'NO_BRIEF_SELECTED'
|
||||
} as const;
|
||||
|
||||
export type ErrorCode = (typeof ERROR_CODES)[keyof typeof ERROR_CODES];
|
||||
|
||||
@@ -161,6 +161,16 @@ export class TaskService {
|
||||
storageType: this.getStorageType()
|
||||
};
|
||||
} catch (error) {
|
||||
// If it's a user-facing error (like NO_BRIEF_SELECTED), don't log it as an internal error
|
||||
if (
|
||||
error instanceof TaskMasterError &&
|
||||
error.is(ERROR_CODES.NO_BRIEF_SELECTED)
|
||||
) {
|
||||
// Just re-throw user-facing errors without wrapping
|
||||
throw error;
|
||||
}
|
||||
|
||||
// Log internal errors
|
||||
this.logger.error('Failed to get task list', error);
|
||||
throw new TaskMasterError(
|
||||
'Failed to get task list',
|
||||
@@ -186,6 +196,14 @@ export class TaskService {
|
||||
// Delegate to storage layer which handles the specific logic for tasks vs subtasks
|
||||
return await this.storage.loadTask(String(taskId), activeTag);
|
||||
} catch (error) {
|
||||
// If it's a user-facing error (like NO_BRIEF_SELECTED), don't wrap it
|
||||
if (
|
||||
error instanceof TaskMasterError &&
|
||||
error.is(ERROR_CODES.NO_BRIEF_SELECTED)
|
||||
) {
|
||||
throw error;
|
||||
}
|
||||
|
||||
throw new TaskMasterError(
|
||||
`Failed to get task ${taskId}`,
|
||||
ERROR_CODES.STORAGE_ERROR,
|
||||
@@ -522,6 +540,14 @@ export class TaskService {
|
||||
activeTag
|
||||
);
|
||||
} catch (error) {
|
||||
// If it's a user-facing error (like NO_BRIEF_SELECTED), don't wrap it
|
||||
if (
|
||||
error instanceof TaskMasterError &&
|
||||
error.is(ERROR_CODES.NO_BRIEF_SELECTED)
|
||||
) {
|
||||
throw error;
|
||||
}
|
||||
|
||||
throw new TaskMasterError(
|
||||
`Failed to update task status for ${taskIdStr}`,
|
||||
ERROR_CODES.STORAGE_ERROR,
|
||||
|
||||
@@ -37,6 +37,13 @@ export interface ApiStorageConfig {
|
||||
maxRetries?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Auth context with a guaranteed briefId
|
||||
*/
|
||||
type ContextWithBrief = NonNullable<
|
||||
ReturnType<typeof AuthManager.prototype.getContext>
|
||||
> & { briefId: string };
|
||||
|
||||
/**
|
||||
* ApiStorage implementation using repository pattern
|
||||
* Provides flexibility to swap between different backend implementations
|
||||
@@ -126,7 +133,7 @@ export class ApiStorage implements IStorage {
|
||||
private async loadTagsIntoCache(): Promise<void> {
|
||||
try {
|
||||
const authManager = AuthManager.getInstance();
|
||||
const context = await authManager.getContext();
|
||||
const context = authManager.getContext();
|
||||
|
||||
// If we have a selected brief, create a virtual "tag" for it
|
||||
if (context?.briefId) {
|
||||
@@ -158,15 +165,7 @@ export class ApiStorage implements IStorage {
|
||||
await this.ensureInitialized();
|
||||
|
||||
try {
|
||||
const authManager = AuthManager.getInstance();
|
||||
const context = await authManager.getContext();
|
||||
|
||||
// If no brief is selected in context, throw an error
|
||||
if (!context?.briefId) {
|
||||
throw new Error(
|
||||
'No brief selected. Please select a brief first using: tm context brief <brief-id>'
|
||||
);
|
||||
}
|
||||
const context = this.ensureBriefSelected('loadTasks');
|
||||
|
||||
// Load tasks from the current brief context with filters pushed to repository
|
||||
const tasks = await this.retryOperation(() =>
|
||||
@@ -181,12 +180,11 @@ export class ApiStorage implements IStorage {
|
||||
|
||||
return tasks;
|
||||
} catch (error) {
|
||||
throw new TaskMasterError(
|
||||
'Failed to load tasks from API',
|
||||
ERROR_CODES.STORAGE_ERROR,
|
||||
{ operation: 'loadTasks', tag, context: 'brief-based loading' },
|
||||
error as Error
|
||||
);
|
||||
this.wrapError(error, 'Failed to load tasks from API', {
|
||||
operation: 'loadTasks',
|
||||
tag,
|
||||
context: 'brief-based loading'
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -237,16 +235,17 @@ export class ApiStorage implements IStorage {
|
||||
await this.ensureInitialized();
|
||||
|
||||
try {
|
||||
this.ensureBriefSelected('loadTask');
|
||||
|
||||
return await this.retryOperation(() =>
|
||||
this.repository.getTask(this.projectId, taskId)
|
||||
);
|
||||
} catch (error) {
|
||||
throw new TaskMasterError(
|
||||
'Failed to load task from API',
|
||||
ERROR_CODES.STORAGE_ERROR,
|
||||
{ operation: 'loadTask', taskId, tag },
|
||||
error as Error
|
||||
);
|
||||
this.wrapError(error, 'Failed to load task from API', {
|
||||
operation: 'loadTask',
|
||||
taskId,
|
||||
tag
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -325,7 +324,7 @@ export class ApiStorage implements IStorage {
|
||||
|
||||
try {
|
||||
const authManager = AuthManager.getInstance();
|
||||
const context = await authManager.getContext();
|
||||
const context = authManager.getContext();
|
||||
|
||||
// In our API-based system, we only have one "tag" at a time - the current brief
|
||||
if (context?.briefId) {
|
||||
@@ -510,6 +509,8 @@ export class ApiStorage implements IStorage {
|
||||
await this.ensureInitialized();
|
||||
|
||||
try {
|
||||
this.ensureBriefSelected('updateTaskStatus');
|
||||
|
||||
const existingTask = await this.retryOperation(() =>
|
||||
this.repository.getTask(this.projectId, taskId)
|
||||
);
|
||||
@@ -546,12 +547,12 @@ export class ApiStorage implements IStorage {
|
||||
taskId
|
||||
};
|
||||
} catch (error) {
|
||||
throw new TaskMasterError(
|
||||
'Failed to update task status via API',
|
||||
ERROR_CODES.STORAGE_ERROR,
|
||||
{ operation: 'updateTaskStatus', taskId, newStatus, tag },
|
||||
error as Error
|
||||
);
|
||||
this.wrapError(error, 'Failed to update task status via API', {
|
||||
operation: 'updateTaskStatus',
|
||||
taskId,
|
||||
newStatus,
|
||||
tag
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -769,6 +770,29 @@ export class ApiStorage implements IStorage {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensure a brief is selected in the current context
|
||||
* @returns The current auth context with a valid briefId
|
||||
*/
|
||||
private ensureBriefSelected(operation: string): ContextWithBrief {
|
||||
const authManager = AuthManager.getInstance();
|
||||
const context = authManager.getContext();
|
||||
|
||||
if (!context?.briefId) {
|
||||
throw new TaskMasterError(
|
||||
'No brief selected',
|
||||
ERROR_CODES.NO_BRIEF_SELECTED,
|
||||
{
|
||||
operation,
|
||||
userMessage:
|
||||
'No brief selected. Please select a brief first using: tm context brief <brief-id> or tm context brief <brief-url>'
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
return context as ContextWithBrief;
|
||||
}
|
||||
|
||||
/**
|
||||
* Retry an operation with exponential backoff
|
||||
*/
|
||||
@@ -787,4 +811,28 @@ export class ApiStorage implements IStorage {
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Wrap an error unless it's already a NO_BRIEF_SELECTED error
|
||||
*/
|
||||
private wrapError(
|
||||
error: unknown,
|
||||
message: string,
|
||||
context: Record<string, unknown>
|
||||
): never {
|
||||
// If it's already a NO_BRIEF_SELECTED error, don't wrap it
|
||||
if (
|
||||
error instanceof TaskMasterError &&
|
||||
error.is(ERROR_CODES.NO_BRIEF_SELECTED)
|
||||
) {
|
||||
throw error;
|
||||
}
|
||||
|
||||
throw new TaskMasterError(
|
||||
message,
|
||||
ERROR_CODES.STORAGE_ERROR,
|
||||
context,
|
||||
error as Error
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -201,6 +201,44 @@ export class TaskMasterCore {
|
||||
return this.taskService.getStorageType();
|
||||
}
|
||||
|
||||
/**
|
||||
* Get storage configuration
|
||||
*/
|
||||
getStorageConfig() {
|
||||
return this.configManager.getStorageConfig();
|
||||
}
|
||||
|
||||
/**
|
||||
* Get storage display information for headers
|
||||
* Returns context info for API storage, null for file storage
|
||||
*/
|
||||
getStorageDisplayInfo(): {
|
||||
briefId: string;
|
||||
briefName: string;
|
||||
orgSlug?: string;
|
||||
} | null {
|
||||
// Only return info if using API storage
|
||||
const storageType = this.getStorageType();
|
||||
if (storageType !== 'api') {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Get credentials from auth manager
|
||||
const authManager = AuthManager.getInstance();
|
||||
const credentials = authManager.getCredentials();
|
||||
const selectedContext = credentials?.selectedContext;
|
||||
|
||||
if (!selectedContext?.briefId || !selectedContext?.briefName) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return {
|
||||
briefId: selectedContext.briefId,
|
||||
briefName: selectedContext.briefName,
|
||||
orgSlug: selectedContext.orgSlug
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Get current active tag
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user