Skip to content

fix(plain): preserve real ForbiddenError message from Plain API#2167

Merged
arnestrickmann merged 1 commit into
generalaction:mainfrom
tobi110289:fix/plain-preserve-forbidden-error
May 21, 2026
Merged

fix(plain): preserve real ForbiddenError message from Plain API#2167
arnestrickmann merged 1 commit into
generalaction:mainfrom
tobi110289:fix/plain-preserve-forbidden-error

Conversation

@tobi110289
Copy link
Copy Markdown
Contributor

@tobi110289 tobi110289 commented May 21, 2026

Summary

PlainConnectionService.validateToken was catching the ForbiddenError thrown by @team-plain/graphql and re-throwing a new one with a hardcoded message:

'Insufficient permissions: this key cannot read threads. Ensure thread read permissions are enabled.'

That overwrote the SDK's original message, which already contained the specific permission detail Plain's API returned (e.g. customer:read, thread:list, workspace-scope issues, etc. — see node_modules/@team-plain/graphql/dist/graphql-client.js:30).

The result: users with valid thread:read permissions but a different missing scope got a misleading "ensure thread read permissions are enabled" error, sending them on a wild goose chase verifying a permission they already had.

This PR drops the rewrite and lets the SDK's ForbiddenError propagate. toPlainErrorMessage already returns error.message first, so the actual Plain-side reason now reaches the settings UI.

Test plan

  • pnpm run format
  • pnpm run lint
  • pnpm run typecheck
  • pnpm run test (823 passed)
  • Manually verified: connecting a Plain key with a missing non-thread permission now surfaces Plain's actual error string in the integrations card

🤖 Generated with Claude Code

`validateToken` was catching the `ForbiddenError` thrown by `@team-plain/graphql`
and re-throwing a new one with a hardcoded "this key cannot read threads"
message, discarding the specific permission detail that Plain's API returned
in the original error.

This led to misleading UI feedback: keys missing unrelated permissions (e.g.
`customer:read`) were reported as missing thread read access. Now the SDK's
`ForbiddenError` propagates unchanged, so `toPlainErrorMessage` surfaces the
exact reason Plain provided.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 21, 2026

Greptile Summary

This PR fixes a misleading error message in PlainConnectionService.validateToken where a caught ForbiddenError was re-thrown with a hardcoded thread-permission message, overwriting the SDK's original, specific reason (e.g. customer:read missing). The fix lets the original error propagate and also trims an inaccurate hint from the toPlainErrorMessage fallback string.

  • validateToken catch block: The special ForbiddenError branch that replaced the message with a hardcoded string is removed; all known Plain SDK error types now re-throw directly in a single combined condition.
  • toPlainErrorMessage fallback: The ForbiddenError fallback is simplified to a generic permissions message, dropping the "thread read permissions" hint that could send users chasing the wrong permission.

Confidence Score: 5/5

Safe to merge — the change is a targeted removal of an error-message rewrite, and error propagation through the existing call chain is logically unchanged for all other error types.

The diff is minimal and well-scoped: one branch that was incorrectly wrapping a ForbiddenError is removed, and a misleading fallback hint is cleaned up. The consolidated instanceof guard in validateToken is logically equivalent to the previous four separate checks for all cases. toPlainErrorMessage already preferred error.message first, so the SDK's original reason now flows through to the UI without further change.

No files require special attention.

Important Files Changed

Filename Overview
src/main/core/plain/plain-connection-service.ts Removes the ForbiddenError re-wrap in validateToken so the SDK's original message propagates, and simplifies the fallback string in toPlainErrorMessage to drop the misleading thread-permissions hint.

Sequence Diagram

sequenceDiagram
    participant UI as Settings UI
    participant SVC as PlainConnectionService
    participant SDK as @team-plain/graphql

    UI->>SVC: saveToken(key)
    SVC->>SDK: "client.query.threads({ first: 1 })"
    SDK-->>SVC: throws ForbiddenError("customer:read missing")

    note over SVC: BEFORE: re-wrap with hardcoded message
    note over SVC: AFTER: re-throw original ForbiddenError

    SVC->>SVC: toPlainErrorMessage(error, fallback)
    note over SVC: returns error.message ("customer:read missing")
    SVC-->>UI: "{ success: false, error: "customer:read missing" }"
Loading

Reviews (1): Last reviewed commit: "fix(plain): preserve real ForbiddenError..." | Re-trigger Greptile

@arnestrickmann arnestrickmann self-requested a review May 21, 2026 19:19
Copy link
Copy Markdown
Contributor

@arnestrickmann arnestrickmann left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Copy Markdown
Contributor

@arnestrickmann arnestrickmann left a comment

Choose a reason for hiding this comment

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

thank you, lgtm

@arnestrickmann arnestrickmann merged commit 1d4350e into generalaction:main May 21, 2026
1 check passed
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