Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make route config errors more prominent #20847

Merged
merged 5 commits into from
Jan 16, 2025

Conversation

tepi
Copy link
Contributor

@tepi tepi commented Jan 15, 2025

When route configuration errors happen, make sure to output the actual error message as the first output at ERROR level. When this is detected at startup, we can't prevent printing out the stack trace too since the container does that when initialization throws. Also checked all the messages for different route misconfiguration cases, and they seem to be fine.

Fixes #20589

Copy link

github-actions bot commented Jan 15, 2025

Test Results

1 163 files  ± 0  1 163 suites  ±0   1h 33m 32s ⏱️ - 3m 30s
7 620 tests ± 0  7 564 ✅ ± 0  56 💤 ±0  0 ❌ ±0 
7 918 runs   - 68  7 855 ✅  - 66  63 💤  - 2  0 ❌ ±0 

Results for commit 51a0e32. ± Comparison against base commit f0dbe87.

♻️ This comment has been updated with latest results.

caalador
caalador previously approved these changes Jan 15, 2025
Copy link
Contributor

@caalador caalador left a comment

Choose a reason for hiding this comment

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

Is fine in devMode when adding/renaming a route that conflicts with server running, but gets hidden on clean devMode start as the routes.tsx is generated later and the exception gets thrown.

@@ -791,9 +791,9 @@ private RuntimeException ambigousTarget(Class<? extends Component> target) {

String messageFormat;
if (isParameter()) {
messageFormat = "Navigation targets must have unique routes, found navigation targets '%s' and '%s' with parameter have the same route.";
messageFormat = "Navigation target paths (considering @Route, @RouteAlias and @RoutePrefix values) must be unique, found navigation targets '%s' and '%s' with parameter having the same route.";
Copy link
Contributor

Choose a reason for hiding this comment

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

As 9 tests failed after updating the message perhaps it would make sense to have these as constants that are also used in the tests.

@caalador caalador merged commit 049bfdf into main Jan 16, 2025
26 checks passed
@caalador caalador deleted the fix/make-route-config-errors-more-prominent branch January 16, 2025 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error message from duplicate routes could be more helpful
3 participants