fix: show actual API error messages for 403 Forbidden responses #572
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request improves error message handling for 403 Forbidden responses by showing actual API error messages instead of generic or misleading messages.
Changes:
- Modified ResponseHelper.java to display actual API error messages for 403 responses using decodeMessage()
- Removed misleading 403→NotFoundException conversions from 17 command files across runs, credentials, computeenvs, and studios modules
- Cleaned up unused exception imports while preserving those still needed in the codebase
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/io/seqera/tower/cli/utils/ResponseHelper.java | Changed 403 error handling to use decodeMessage() for actual API error messages |
| src/main/java/io/seqera/tower/cli/commands/runs/ViewCmd.java | Removed try-catch that converted 403 to RunNotFoundException |
| src/main/java/io/seqera/tower/cli/commands/runs/DeleteCmd.java | Removed 403→RunNotFoundException conversion and unused import |
| src/main/java/io/seqera/tower/cli/commands/runs/CancelCmd.java | Removed 403→RunNotFoundException conversion and unused import |
| src/main/java/io/seqera/tower/cli/commands/credentials/DeleteCmd.java | Removed 403→CredentialsNotFoundException conversion and unused import |
| src/main/java/io/seqera/tower/cli/commands/credentials/update/AbstractUpdateCmd.java | Removed 403→CredentialsNotFoundException conversion and unused import |
| src/main/java/io/seqera/tower/cli/commands/computeenvs/ViewCmd.java | Removed 403→ComputeEnvNotFoundException conversion and unused import |
| src/main/java/io/seqera/tower/cli/commands/computeenvs/UpdateCmd.java | Simplified describeCE method signature, removed 403 handling and unused import |
| src/main/java/io/seqera/tower/cli/commands/computeenvs/DeleteCmd.java | Removed 403→ComputeEnvNotFoundException conversion and unused import |
| src/main/java/io/seqera/tower/cli/commands/studios/ViewCmd.java | Removed generic 403 error message, kept TowerException import removed |
| src/main/java/io/seqera/tower/cli/commands/studios/TemplatesCmd.java | Removed entire try-catch block for 403, removed unused import |
| src/main/java/io/seqera/tower/cli/commands/studios/StopCmd.java | Removed generic 403 error message, removed unused import |
| src/main/java/io/seqera/tower/cli/commands/studios/StartCmd.java | Removed generic 403 error message, removed unused import |
| src/main/java/io/seqera/tower/cli/commands/studios/ListCmd.java | Removed generic 403 error message, removed unused import |
| src/main/java/io/seqera/tower/cli/commands/studios/DeleteCmd.java | Removed generic 403 error message, removed unused import |
| src/main/java/io/seqera/tower/cli/commands/studios/CheckpointsCmd.java | Removed generic 403 error message, removed unused import |
| src/main/java/io/seqera/tower/cli/commands/studios/AddCmd.java | Removed try-catch for 403, properly kept TowerException import (still used) |
| src/main/java/io/seqera/tower/cli/commands/studios/AddAsNewCmd.java | Removed try-catch for 403, properly kept TowerException import (still used) |
| docs/plans/2026-01-19-verbose-403-error-messages.md | Added comprehensive implementation plan documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/io/seqera/tower/cli/commands/credentials/update/AbstractUpdateCmd.java
Show resolved
Hide resolved
|
Heya @ejseqera. I think the error messages here are more accurate and informative. I want to check with you first - would changing the text in these error messages break something in Seqerakit? |
- Update ResponseHelper to display the actual API error message for 403 responses - Remove misleading 403->NotFoundException conversions across commands: - credentials (delete, update) - runs (delete, cancel, view) - computeenvs (view, delete, update) - studios (view, list, checkpoints, templates, stop, delete, start, add, add-as-new) - Update tests to expect the new 403 error behavior Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Rob Syme <rob.syme@gmail.com>
robsyme
force-pushed
the
more-verbose-error-message
branch
from
bbb082d to
a7bf83a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/test/java/io/seqera/tower/cli/credentials/providers/AwsProviderTest.java
Show resolved
Hide resolved
Signed-off-by: Rob Syme <rob.syme@gmail.com>
weronikasosnowskaseqera
left a comment
weronikasosnowskaseqera
left a comment
There was a problem hiding this comment.
For me It's fine but I wonder if @JaimeSeqLabs or @tcrespog know why we decided to hide those error messages in the first place?
I can only guess that it was meant to hide the fact that the some resource being accessed actually exist (so |
Summary
ResponseHelper.javato display actual API error messages for 403 responses instead of generic "Unknown. Check that the provided identifier is correct."Forbidden. Missing permission(s): "organization:write"instead of confusing "not found" errorsTest Plan
tw -v credentials delete <id>with insufficient permissions and verify actual permission error is showntw -v studios view -i <id>with insufficient permissions and verify actual permission error is shownThis resolves some misleading responses. For example, if a Platform org with insuffficient permissions tried to list all org members, we would respond with:
but these changes now adjust that message to the more helpful: