fix: surface batch API errors in commands#159
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses cases where CLI commands assume batched API responses always contain successful .data payloads, causing misleading internal crashes on API validation/request errors. It centralizes batch-response validation so commands surface the real API error message instead.
Changes:
- Enhance
assertBatchDatato extract and display batch API error details (e.g.,errorString) viaCliError('API_ERROR'). - Introduce
buildBatchNameMapand refactor inbox/thread/conversation commands to use it (andassertBatchData) instead of directly accessingresponse.data. - Add regression tests ensuring batch failures (including per-entity user lookups) produce clear errors rather than
TypeErrors.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/lib/api.ts |
Adds batch error-message extraction, strengthens assertBatchData, and introduces buildBatchNameMap to safely consume batch results. |
src/commands/thread/view.ts |
Uses shared helpers to validate batch responses and fail fast with clear errors on user lookup failures. |
src/commands/inbox.ts |
Validates inbox/unread batch results and uses buildBatchNameMap for channel name resolution, preventing .data.map crashes. |
src/commands/conversation/view.ts |
Validates conversation/messages batch results and uses buildBatchNameMap for participant names. |
src/commands/conversation/unread.ts |
Validates per-conversation batch lookups and uses buildBatchNameMap for participant names. |
src/commands/conversation/helpers.ts |
Switches user-name map construction to buildBatchNameMap for consistent batch validation behavior. |
src/__tests__/thread.test.ts |
Adds coverage for failed batched user lookup during thread view. |
src/__tests__/inbox.test.ts |
Adds coverage for surfacing a batch API validation error (limit too high) in inbox. |
src/__tests__/conversation.test.ts |
Adds coverage for failed batched user lookup during conversation view. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
doistbot
reviewed
Apr 8, 2026
Member
doistbot
left a comment
doistbot
left a comment
There was a problem hiding this comment.
This PR updates the CLI commands to properly surface batch API errors, improving observability and error awareness. However, the updated user lookup logic in the thread view inadvertently causes a hard failure for missing user entries instead of preserving the existing fallback behavior that gracefully skips null results.
scottlovegrove
approved these changes
Apr 9, 2026
scottlovegrove
force-pushed
the
fix/158-surface-batch-errors
branch
from
cbdd9ec to
9b98e0b
Compare
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`buildBatchNameMap` was applied to user lookups in thread and conversation views, which made a single deleted/missing user abort the whole command. Add `buildOptionalBatchNameMap` that skips entries with `data: null` and a success code while still surfacing real API errors, and use it for user maps. Callers continue to fall back to `user:<id>` for missing entries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doist-release-bot Bot
added a commit
that referenced
this pull request
May 13, 2026
## [2.36.4](v2.36.3...v2.36.4) (2026-05-13) ### Bug Fixes * surface batch API errors in commands ([#159](#159)) ([3b30c3c](3b30c3c))
Contributor
|
🎉 This PR is included in version 2.36.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
scottlovegrove
added a commit
that referenced
this pull request
May 13, 2026
…/inbox Address review feedback on PR #224 now that #159 is merged: - Add `getOptionalBatchData` helper in `src/lib/api.ts` — returns `data` (or null) on success, throws via `assertBatchData` only on real API errors. Use for batch sub-requests where `data: null` with a success code is a valid empty state (e.g. `getUnread` when the workspace has zero unread threads). - `channel/threads.ts`: use `assertBatchData` for primary `threadsResp` so a failed `getThreads` sub-request surfaces an API error instead of crashing with `TypeError: .map is not a function`. Use `getOptionalBatchData` for `unreadResp` to preserve the "no unread = empty" valid state. - `inbox.ts`: switch `unreadResponse` from strict `assertBatchData` to `getOptionalBatchData` (fixes regression introduced by #159 where a workspace with zero unread threads would crash `tw inbox`). - `channel/threads.test.ts`: update `setupClient` to accept `unread: null`, add `code: 200` to batch mock fixtures, refactor the regression test to use the helper, and add a new test for the threads-batch-error path. - `inbox.test.ts`: add a regression test for the null-unread case. - Document the four batch helpers and when to use each in AGENTS.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #158. The issue includes a repro.