feat(new-webui): Add backend support for log viewer links in search results. #950
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Warning Rate limit exceeded@davemarco has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 31 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughThis change updates the log viewer web UI to support internal navigation for viewing log streams. It introduces new route handling for Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchResultsTable
participant Message
participant LogViewerLink
participant Router
participant QueryStatus
User->>SearchResultsTable: Clicks log message link
SearchResultsTable->>Message: Render with streamId, logEventIdx, filePath
Message->>LogViewerLink: Render with streamId, logEventIdx, filePath
LogViewerLink->>Router: Navigate to /streamFile?type=...&streamId=...&logEventIdx=...
Router->>QueryStatus: Render QueryStatus component for /streamFile route
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
junhaoliao
requested changes
May 31, 2025
...webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx
Outdated
Show resolved
Hide resolved
...webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx
Outdated
Show resolved
Hide resolved
...webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx
Outdated
Show resolved
Hide resolved
...-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/index.tsx
Outdated
Show resolved
Hide resolved
...ts/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx
Show resolved
Hide resolved
...nents/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/utils.ts
Outdated
Show resolved
Hide resolved
|
discussed offline - since the UIs (links) are already there, for the PR title, how about: |
davemarco
changed the title
|
@junhaoliao - i still need approval |
junhaoliao
approved these changes
Jun 2, 2025
3 tasks
junhaoliao
reviewed
Jun 15, 2025
Comment on lines
+55
to
57
| fastify.get("/streamFile", (_, reply) => { | ||
| reply.sendFile("index.html", clientDir); | ||
| }); |
There was a problem hiding this comment.
to get the other React-Router defined routes in the SPA working, i believe we also need to let Fastify serve those routes with the index.html
@coderabbitai please create an issue to track this
There was a problem hiding this comment.
🧩 Analysis chain
@junhaoliao I'll create an issue to track the SPA routing problem you've identified.
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue to track the SPA routing problem
gh issue create \
--title "SPA routing: Fastify needs to serve index.html for all React Router routes" \
--body "## Problem
Currently, only the \`/streamFile\` route is configured to serve \`index.html\` in the Fastify server. Other React Router defined routes in the SPA will return 404 errors when accessed directly via URL or when users refresh the page.
## Current Implementation
In \`components/log-viewer-webui/server/src/routes/static.ts\`, we have:
- Static file serving with \`wildcard: false\`
- Specific route handler for \`/streamFile\` that serves \`index.html\`
## Expected Behavior
All React Router defined routes should be served with \`index.html\` so that the React application can handle client-side routing properly.
## Related
- PR: https://github.com/y-scope/clp/pull/950
- Comment: https://github.com/y-scope/clp/pull/950#discussion_r2148823183
- Requested by: @junhaoliao" \
--assignee junhaoliao
Length of output: 952
@junhaoliao I've successfully created issue #1011 to track the SPA routing problem: #1011
The issue documents the current implementation where only /streamFile serves index.html, and explains that other React Router routes will need similar handling to prevent 404 errors when accessed directly or on page refresh. The issue has been assigned to you as requested.
🐰📝
Taking notes for future improvements
🧬 Code Graph Analysis Results
Relevant snippet from components/log-viewer-webui/server/src/routes/static.ts (lines 1-56)
import path from "node:path";
import process from "node:process";
import {fileURLToPath} from "node:url";
import {fastifyStatic} from "@fastify/static";
import {FastifyPluginAsync} from "fastify";
import settings from "../../settings.json" with {type: "json"};
/**
* Creates static files serving routes.
*
* @param fastify
*/
const routes: FastifyPluginAsync = async (fastify) => {
const filename = fileURLToPath(import.meta.url);
const dirname = path.dirname(filename);
const rootDirname = path.resolve(dirname, "../..");
let streamFilesDir = settings.StreamFilesDir;
if (false === path.isAbsolute(streamFilesDir)) {
streamFilesDir = path.resolve(rootDirname, streamFilesDir);
}
await fastify.register(fastifyStatic, {
prefix: "/streams",
root: streamFilesDir,
});
let logViewerDir = settings.LogViewerDir;
if (false === path.isAbsolute(logViewerDir)) {
logViewerDir = path.resolve(rootDirname, logViewerDir);
}
await fastify.register(fastifyStatic, {
prefix: "/log-viewer",
root: logViewerDir,
decorateReply: false,
});
if ("production" === process.env.NODE_ENV) {
// In the development environment, we expect the client to use a separate webserver that
// supports live reloading.
let clientDir = settings.ClientDir;
if (false === path.isAbsolute(clientDir)) {
clientDir = path.resolve(rootDirname, settings.ClientDir);
}
await fastify.register(fastifyStatic, {
prefix: "/",
root: clientDir,
decorateReply: false,
wildcard: false,
});
fastify.get("/streamFile", (_, reply) => {
reply.sendFile("index.html", clientDir);
});
}
};
export default routes;Summary of possibly relevant snippet from components/webui/server/main.js (lines 30-33)
const {
CLP_DB_USER,
CLP_DB_PASS,
} = process.env;- This snippet extracts environment variables related to database credentials from
process.env. - It is a small part of the server main file, likely related to configuration.
- No direct relation to static file serving or React Router routes handling.
No other snippets provided appear relevant to the static file serving or SPA route handling discussed in the user comment.
This was referenced Jun 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Host old log viewer status page, so link to log viewer actually work. Also passes in the neccesary arguments.
Checklist
breaking change.
Validation performed
Tested with another instance of the package, and links opened correctly
Had to modify this script to actually host the new app.
Summary by CodeRabbit
New Features
Bug Fixes