fix: handle 404 redirection on root route#7454
Conversation
|
Welcome to the SigNoz community! Thank you for your first pull request and making this project better. 🤗 |
|
Aditya Singh seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 46bcae6 in 1 minute and 20 seconds
More details
- Looked at
12lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. frontend/src/AppRoutes/index.tsx:313
- Draft comment:
Added route for '/' now returns null to avoid falling into the NotFound route. Consider adding a comment to explain why returning null (e.g., if redirection occurs elsewhere) or explicitly using a if a target route is intended. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
2. frontend/src/AppRoutes/index.tsx:270
- Draft comment:
The comment on line 270 says "if the required calls fails then return a something went wrong error". Consider changing it to "if the required calls fail then return a 'something went wrong' error" for correct subject-verb agreement. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. frontend/src/AppRoutes/index.tsx:272
- Draft comment:
The comment on line 272 contains the word "indefinitive". It might be clearer to use "indefinite" (or another appropriate term) to describe the loading state. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_AtqPgPjXcKbQzeDO
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on d815c84 in 27 seconds
More details
- Looked at
21lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. frontend/src/AppRoutes/index.tsx:310
- Draft comment:
Good change replacing a null render with the Home component. Ensure that the Home component correctly manages redirection logic so that the root route never lands on a 404 page. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. frontend/src/AppRoutes/index.tsx:310
- Draft comment:
Good fix: replacing the null render with the Home component for the '/' route resolves the 404 flicker. Ensure that the Home component handles redirection (if needed) or renders the correct landing page, so that the user experience remains smooth. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. frontend/src/AppRoutes/index.tsx:272
- Draft comment:
Typographical issue: In the comment on lines 271-273, consider changing 'if the required calls fails' to 'if the required calls fail' for proper subject-verb agreement. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. frontend/src/AppRoutes/index.tsx:273
- Draft comment:
Typographical issue: In the comment on lines 271-273, consider replacing 'indefinitive loading' with 'indefinite loading' to correct the typo. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_9AkuzVMIscfPv7av
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Summary
This fixes the intermittent "Not Found" screen that appears on the root route before the appropriate redirection is triggered.
Related Issues / PR's
fixes: #7384
Screenshots
NA
Affected Areas and Manually Tested Areas
Important
Adds a root path route in
AppRoutes/index.tsxto renderHomeand prevent 404 errors before redirection.Routefor the root path/inAppRoutes/index.tsxto render theHomecomponent.HomefrompageComponentsinAppRoutes/index.tsx.This description was created by
for d815c84. It will automatically update as commits are pushed.