Skip to content

Event approach: clean up API error handling with concise user messages#115

Open
tareqas wants to merge 1 commit into
mainfrom
handler
Open

Event approach: clean up API error handling with concise user messages#115
tareqas wants to merge 1 commit into
mainfrom
handler

Conversation

@tareqas

@tareqas tareqas commented Sep 10, 2025

Copy link
Copy Markdown
Member

No description provided.

@tareqas
tareqas force-pushed the handler branch 2 times, most recently from 242b210 to 5cd82df Compare September 10, 2025 15:31
Comment thread Sdk/Api.php Outdated
$output->writeln('<error>Something went wrong. Please try again.</error>');
}

$event->setExitCode(0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

0?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It will terminate the process gracefully and display only a user-friendly message. No exception details or additional output.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

correct, but is that what ppl will expect? I'm not sure

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

exit code is 0

➜  insight git:(handler) ✗ bin/insight analyze 1c92dfd89776     
Something went wrong. Please try again, or run with -v for more details.

exit code 1

➜  insight git:(handler) ✗ bin/insight analyze 1c92dfd89776
Something went wrong. Please try again, or run with -v for more details.

In AnalyzeCommand.php line 43:
                                  
  Testing the exception handling  
                                  

analyze [--format FORMAT] [--reference REFERENCE] [--branch BRANCH] [--show-ignored-violations] [--poll-period POLL-PERIOD] [--fail-condition FAIL-CONDITION] [--no-wait] [--] <project-uuid>

@nicolas-grekas nicolas-grekas Sep 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This doesn't make sense. Optimizing the displayed output shouldn't come at the price of breaking exit codes. Can you figure out another way that doesn't break the exit code?

Comment thread Cli/EventListener/ApiErrorListener.php
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