Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/neat-badgers-matter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@hyperdx/common-utils": patch
"@hyperdx/api": patch
"@hyperdx/app": patch
---

feat: move rrweb event fetching to the client instead of an api route
1 change: 0 additions & 1 deletion packages/api/src/api-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ app.use('/', routers.rootRouter);
app.use('/alerts', isUserAuthenticated, routers.alertsRouter);
app.use('/dashboards', isUserAuthenticated, routers.dashboardRouter);
app.use('/me', isUserAuthenticated, routers.meRouter);
app.use('/sessions', isUserAuthenticated, routers.sessionsRouter);
app.use('/team', isUserAuthenticated, routers.teamRouter);
app.use('/webhooks', isUserAuthenticated, routers.webhooksRouter);
app.use('/datasources', isUserAuthenticated, routers.datasourceRouter);
Expand Down
17 changes: 17 additions & 0 deletions packages/api/src/middleware/validation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import express from 'express';
import { z } from 'zod';

export function validateRequestHeaders<T extends z.Schema>(schema: T) {
return function (
req: express.Request,
res: express.Response,
next: express.NextFunction,
) {
const parsed = schema.safeParse(req.headers);
if (!parsed.success) {
return res.status(400).json({ type: 'Headers', errors: parsed.error });
}

return next();
};
}
44 changes: 29 additions & 15 deletions packages/api/src/routers/api/clickhouseProxy.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import express, { Request, Response } from 'express';
import express, { RequestHandler, Response } from 'express';
import { createProxyMiddleware } from 'http-proxy-middleware';
import { z } from 'zod';
import { validateRequest } from 'zod-express-middleware';

import { getConnectionById } from '@/controllers/connection';
import { getNonNullUserWithTeam } from '@/middleware/auth';
import { validateRequestHeaders } from '@/middleware/validation';
import { objectIdSchema } from '@/utils/zod';

const router = express.Router();
Expand Down Expand Up @@ -58,17 +59,22 @@ router.post(
},
);

router.get(
'/*',
validateRequest({
query: z.object({
hyperdx_connection_id: objectIdSchema,
}),
const hasConnectionId = validateRequestHeaders(
z.object({
'x-hyperdx-connection-id': objectIdSchema,
}),
);

const getConnection: RequestHandler =
// prettier-ignore-next-line
async (req, res, next) => {
try {
const { teamId } = getNonNullUserWithTeam(req);
const { hyperdx_connection_id } = req.query;
const connection_id = req.headers['x-hyperdx-connection-id']!; // ! because zod already validated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Header x-hyperdx-connection-id is now in place of the respective queryparam. I chose this based on the clickhouse documentation for a reverse-proxy in front of a clickhouse instance. They do have a section that allows you to send query params as well, but those are all prefixed with 'param_', which quickly gets messy with zod validating either query param 'hyperdx_connection_id' or 'param_hyperdx_connection_id', so I opted to move to using headers.

delete req.headers['x-hyperdx-connection-id'];
const hyperdx_connection_id = Array.isArray(connection_id)
? connection_id.join('')
: connection_id;

const connection = await getConnectionById(
teamId.toString(),
Expand All @@ -93,13 +99,15 @@ router.get(
console.error('Error fetching connection info:', e);
next(e);
}
},
};

const proxyMiddleware: RequestHandler =
// prettier-ignore-next-line
createProxyMiddleware({
target: '', // doesn't matter. it should be overridden by the router
changeOrigin: true,
pathFilter: (path, _req) => {
// TODO: allow other methods
return _req.method === 'GET';
return _req.method === 'GET' || _req.method === 'POST';
},
pathRewrite: {
'^/clickhouse-proxy': '',
Expand All @@ -113,16 +121,20 @@ router.get(
on: {
proxyReq: (proxyReq, _req) => {
const newPath = _req.params[0];
// @ts-expect-error _req.query is type ParamQs, which doesn't play nicely with URLSearchParams. TODO: Replace with getting query params from _req.url eventually
const qparams = new URLSearchParams(_req.query);
Comment on lines +124 to 125
Copy link

Copilot AI Apr 22, 2025

Choose a reason for hiding this comment

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

Instead of relying on an implicit conversion of _req.query, explicitly converting it by calling toString() or retrieving query params from _req.url would improve clarity and reliability.

Suggested change
// @ts-expect-error _req.query is type ParamQs, which doesn't play nicely with URLSearchParams. TODO: Replace with getting query params from _req.url eventually
const qparams = new URLSearchParams(_req.query);
const url = new URL(_req.url, `http://${_req.headers.host}`);
const qparams = new URLSearchParams(url.search);

Copilot uses AI. Check for mistakes.
qparams.delete('hyperdx_connection_id');
if (_req._hdx_connection?.username && _req._hdx_connection?.password) {
proxyReq.setHeader(
'X-ClickHouse-User',
_req._hdx_connection.username,
);
proxyReq.setHeader('X-ClickHouse-Key', _req._hdx_connection.password);
}
proxyReq.path = `/${newPath}?${qparams.toString()}`;
if (_req.method === 'POST') {
// TODO: Use fixRequestBody after this issue is resolved: https://github.com/chimurai/http-proxy-middleware/issues/1102
proxyReq.write(_req.body);
}
proxyReq.path = `/${newPath}?${qparams}`;
},
proxyRes: (proxyRes, _req, res) => {
// since clickhouse v24, the cors headers * will be attached to the response by default
Expand Down Expand Up @@ -158,7 +170,9 @@ router.get(
// ...(config.IS_DEV && {
// logger: console,
// }),
}),
);
});

router.get('/*', hasConnectionId, getConnection, proxyMiddleware);
router.post('/*', hasConnectionId, getConnection, proxyMiddleware);

export default router;
2 changes: 0 additions & 2 deletions packages/api/src/routers/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import dashboardRouter from './dashboards';
import datasourceRouter from './datasources';
import meRouter from './me';
import rootRouter from './root';
import sessionsRouter from './sessions';
import teamRouter from './team';
import webhooksRouter from './webhooks';

Expand All @@ -13,7 +12,6 @@ export default {
dashboardRouter,
meRouter,
rootRouter,
sessionsRouter,
teamRouter,
webhooksRouter,
};
152 changes: 0 additions & 152 deletions packages/api/src/routers/api/sessions.ts

This file was deleted.

8 changes: 1 addition & 7 deletions packages/app/pages/api/[...all].ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const DEFAULT_SERVER_URL = `http://127.0.0.1:${process.env.HYPERDX_API_PORT}`;
export const config = {
api: {
externalResolver: true,
bodyParser: true,
bodyParser: false,
},
};

Expand All @@ -17,12 +17,6 @@ export default (req: NextApiRequest, res: NextApiResponse) => {
pathRewrite: { '^/api': '' },
target: process.env.NEXT_PUBLIC_SERVER_URL || DEFAULT_SERVER_URL,
autoRewrite: true,
/**
* Fix bodyParser
**/
on: {
proxyReq: fixRequestBody,
},
// ...(IS_DEV && {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The function fixRequestBody does not handle Content-Type: 'text/plain', which makes things horribly annoying. Here's what happens:

  1. bodyParser.text reads the incoming stream to req.body
  2. fixRequestBody looks at the content type and sees 'text/plain', but it doesn't handle that, so it does not write to the proxy request
  3. The proxy request is sent to our express server with a non-zero content-length but an empty body
  4. The express server's express.text middleware (bodyParser under the hood) will see text/plain and a Content-Length > 0, so it tries to read the incoming stream and parse it into req.body
  5. The stream never comes in, so next() is never called. The server will just wait ad infinitum.

I filed an issue and PR to fix fixRequestBody. But we don't even need bodyParser here, so just disabling it and forwarding everything works too.

// logger: console,
// }),
Expand Down
Loading