Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Hotfix/console 404 reporting #2437

Conversation

weierophinney
Copy link
Member

Adds error reporting capabilities to the Console RouteNotFoundStrategy.

If the view_manager "display_not_found_reason" flag is enabled, this will display the reason for the "404" condition, as well as any exceptions, if any, present in the event (looping through all previous exceptions).

@ghost ghost assigned EvanDotPro Sep 27, 2012
$result = $banner ? rtrim($banner, "\r\n") : '';
$result .= $usage ? "\n\n" . trim($usage, "\r\n") : '';
$result .= "\n"; // to ensure we output a final newline

// Report 404 reason and/or exceptions
Copy link
Member

Choose a reason for hiding this comment

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

This method hereby becomes 100+ lines long. Can't we move this to its own method?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

The getConsoleUsage() method has 100+ lines too 😔

Copy link
Member Author

Choose a reason for hiding this comment

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

@Freeaqingme Done.

@b-durand I didn't add any code to getConsoleUsage(), which makes that out-of-scope for this PR.

- If display_not_found_reason is enabled, use this value to display:
  - reason for "404"
  - any related exceptions
- so that we can determine full root cause from CLI
- extracted display_not_found_reason and exception reporting to
  reportNotFoundReason() method, per feedback on pull request.
@weierophinney
Copy link
Member Author

I figured a force push would tell travis to re-run. Odd.

Rebased off of latest master, which corrects the CS issues reported in the former travis results. I've also extracted the logic for displaying the exception stack trace to a new method, addressing the concerns @Freeaqingme raised.

@ralphschindler ralphschindler merged commit 52162ad into zendframework:master Oct 5, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants