Skip to content
Merged
32 changes: 30 additions & 2 deletions apps/meteor/server/api/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { type APIActionHandler, RocketChatAPIRouter } from './router';
import { metrics } from '../lib/metrics';
import { settings } from '../settings';
import { cors } from './v1/middlewares/cors';
import { experimentalWarningMiddleware } from './v1/middlewares/experimental';
import { loggerMiddleware } from './v1/middlewares/logger';
import { metricsMiddleware } from './v1/middlewares/metrics';
import { remoteAddressMiddleware } from './v1/middlewares/remoteAddressMiddleware';
Expand Down Expand Up @@ -42,6 +43,7 @@ const createApi = function _createApi(options: { version?: string; useDefaultAut
export const API: {
api: Router<'/api', any, APIActionHandler>;
v1: APIClass<'/v1'>;
experimental: APIClass<'/experimental'>;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
default: APIClass;
ApiClass: typeof APIClass;
channels?: {
Expand Down Expand Up @@ -73,9 +75,18 @@ export const API: {
version: 'v1',
useDefaultAuth: true,
}),
experimental: createApi({
Comment thread
sampaiodiego marked this conversation as resolved.
version: 'experimental',
useDefaultAuth: true,
}),
default: createApi({}),
};

// Stamp the unstable-signal headers on every experimental response. Registered
// here, at module load, so it precedes any endpoint route registered later on
// API.experimental (Hono runs `.use` middleware in registration order).
API.experimental.router.use(experimentalWarningMiddleware());
Comment thread
sampaiodiego marked this conversation as resolved.
Outdated

settings.watch<string>('Accounts_CustomFields', (value) => {
if (!value) {
return API.v1?.setLimitedCustomFields([]);
Expand All @@ -89,14 +100,19 @@ settings.watch<string>('Accounts_CustomFields', (value) => {
}
});

const reloadRoutesToRefreshRateLimiter = () => {
API.v1.reloadRoutesToRefreshRateLimiter();
API.experimental.reloadRoutesToRefreshRateLimiter();
};

settings.watch<number>('API_Enable_Rate_Limiter_Limit_Time_Default', (value) => {
defaultRateLimiterOptions.intervalTimeInMS = value;
API.v1.reloadRoutesToRefreshRateLimiter();
reloadRoutesToRefreshRateLimiter();
});

settings.watch<number>('API_Enable_Rate_Limiter_Limit_Calls_Default', (value) => {
defaultRateLimiterOptions.numRequestsAllowed = value;
API.v1.reloadRoutesToRefreshRateLimiter();
reloadRoutesToRefreshRateLimiter();
});

export const startRestAPI = () => {
Expand All @@ -113,11 +129,23 @@ export const startRestAPI = () => {
activeRequestsGauge: metrics.rocketchatRestApiActiveRequests,
}),
)
.use(
metricsMiddleware({
basePathRegex: new RegExp(/^\/api\/experimental\//),
Comment thread
sampaiodiego marked this conversation as resolved.
api: API.experimental,
settings,
endpointTimeSummary: metrics.rocketchatRestApi,
endpointTimeHistogram: metrics.rocketchatRestApiSeconds,
responseSizeHistogram: metrics.rocketchatRestApiResponseSizeBytes,
activeRequestsGauge: metrics.rocketchatRestApiActiveRequests,
}),
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
.use(tracerSpanMiddleware)
.use(remoteAddressMiddleware)
.use(cors(settings))
.use(loggerMiddleware(logger))
.use(API.v1.router)
.use(API.experimental.router)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
.use(API.default.router).router,
);
};
Expand Down
22 changes: 22 additions & 0 deletions apps/meteor/server/api/v1/middlewares/experimental.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import type { MiddlewareHandler } from 'hono';

// RFC 7234 "miscellaneous persistent warning" code. Paired with the
// programmatic-friendly `x-experimental` header so clients can detect, in code,
// that they hit an unstable endpoint.
const WARNING_HEADER = '299 - "experimental: endpoint is unstable and may change without notice"';

/**
* Stamps every response from the experimental API instance with the unstable
* signal headers. Registered on `API.experimental` only — `/api/v1/*` and the
* default router never see these headers.
*
* Mirrors the header-writing pattern of `writeDeprecationHeader` in
* `deprecationWarningLogger.ts`: the headers are set on `c.res.headers` before
* the route handler runs so they are picked up when the handler builds the
* final response (see `Router.method` in `@rocket.chat/http-router`).
*/
export const experimentalWarningMiddleware = (): MiddlewareHandler => async (c, next) => {
c.res.headers.set('x-experimental', 'true');
Comment thread
sampaiodiego marked this conversation as resolved.
Outdated
c.res.headers.set('Warning', WARNING_HEADER);
await next();
};
87 changes: 87 additions & 0 deletions apps/meteor/server/api/v1/middlewares/metrics.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,4 +199,91 @@ describe('Metrics middleware', () => {
entrypoint: 'method.call/get:param',
});
});

it('should only record requests matching its own base path', async () => {
const ajv = new Ajv();
const app = express();
const settings = new CachedSettings();

const makeMetrics = () => {
const endTimer = jest.fn();
return {
endTimer,
summary: { startTimer: jest.fn().mockReturnValue(endTimer) },
histogram: { startTimer: jest.fn().mockReturnValue(jest.fn()) },
responseSizeHistogram: { observe: jest.fn() },
activeRequestsGauge: { inc: jest.fn(), dec: jest.fn() },
};
};

const v1Metrics = makeMetrics();
const experimentalMetrics = makeMetrics();

const route = (router: Router<any, any, any>) =>
router.get(
'/test',
{
response: {
200: ajv.compile({
type: 'object',
properties: {
message: { type: 'string' },
},
}),
},
},
async () => ({
statusCode: 200,
body: { message: 'Metrics test successful' },
}),
);

const api = new Router('/api');

api
.use(
metricsMiddleware({
basePathRegex: new RegExp(/^\/api\/v1\//),
api: { version: 'v1' } as any,
settings,
endpointTimeSummary: v1Metrics.summary as any,
endpointTimeHistogram: v1Metrics.histogram as any,
responseSizeHistogram: v1Metrics.responseSizeHistogram as any,
activeRequestsGauge: v1Metrics.activeRequestsGauge as any,
}),
)
.use(
metricsMiddleware({
basePathRegex: new RegExp(/^\/api\/experimental\//),
api: { version: 'experimental' } as any,
settings,
endpointTimeSummary: experimentalMetrics.summary as any,
endpointTimeHistogram: experimentalMetrics.histogram as any,
responseSizeHistogram: experimentalMetrics.responseSizeHistogram as any,
activeRequestsGauge: experimentalMetrics.activeRequestsGauge as any,
}),
)
.use(route(new Router('/v1')))
.use(route(new Router('/experimental')));

app.use(api.router);

expect((await request(app).get('/api/v1/test')).statusCode).toBe(200);

expect(v1Metrics.summary.startTimer).toHaveBeenCalledTimes(1);
expect(v1Metrics.endTimer).toHaveBeenCalledWith({ status: 200, method: 'get', version: 'v1', entrypoint: 'test' });
expect(experimentalMetrics.summary.startTimer).not.toHaveBeenCalled();
expect(experimentalMetrics.activeRequestsGauge.inc).not.toHaveBeenCalled();

expect((await request(app).get('/api/experimental/test')).statusCode).toBe(200);

expect(experimentalMetrics.summary.startTimer).toHaveBeenCalledTimes(1);
expect(experimentalMetrics.endTimer).toHaveBeenCalledWith({
status: 200,
method: 'get',
version: 'experimental',
entrypoint: 'test',
});
expect(v1Metrics.summary.startTimer).toHaveBeenCalledTimes(1);
});
});
6 changes: 6 additions & 0 deletions apps/meteor/server/api/v1/middlewares/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ export const metricsMiddleware =
activeRequestsGauge: Gauge;
}): MiddlewareHandler =>
async (c, next) => {
// Several metrics middlewares share the same `/api` mount (v1, experimental, apps), so each
// one has to ignore the paths that belong to the others or a request gets sampled more than once.
if (basePathRegex && !basePathRegex.test(c.req.path)) {
Comment thread
sampaiodiego marked this conversation as resolved.
return next();
}

const rocketchatRestApiEnd = endpointTimeSummary.startTimer();
const rocketchatRestApiHistEnd = endpointTimeHistogram.startTimer();

Expand Down
183 changes: 183 additions & 0 deletions docs/experimental-api-endpoints-plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
# Plan: Experimental REST API endpoints

## Goal

Allow REST endpoints to ship to production but **change or be removed in any release
without a major-version bump**. The mechanism must be general-purpose (any team can
use it), not specific to one feature.

## The contract

> Endpoints under `/api/experimental/...` are unstable. They may change shape or be
> removed in any release, without notice and without a deprecation cycle. No semver
> promise attaches to this namespace.

The namespace **is** the contract. A caller hitting `/api/experimental/*` has opted
into instability by the URL alone. `/v1` keeps its implicit semver-stability promise,
untouched.

**Only the new typed API is allowed on experimental routes.** Endpoints must be
registered with `.get()` / `.post()` / `.put()` / `.delete()` (with AJV `body` / `query`
/ `response` validators). The deprecated `.addRoute()` is **not** available for
experimental endpoints — new surface area should not be born on the legacy registration
path.

## Why this design (key findings from the current code)

- `createApi({ version })` turns the `version` string into the URL path segment, so a
new instance with `version: 'experimental'` mounts at `/api/experimental/<name>`
with zero router changes. See `apps/meteor/app/api/server/api.ts:33-77` and
Comment thread
sampaiodiego marked this conversation as resolved.
Outdated
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
`ApiClass.ts:199`.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
- The typed route methods `.get()/.post()/.put()/.delete()` are generic over
`TSubPathPattern extends string` (`ApiClass.ts:669`) — they are **not** gated on
`keyof Endpoints`. Routes accumulate *outward* into the instance's `TOperations`,
read back by `ExtractApiClassEndpoints` (`ApiClass.ts:125-126`). So a separate
experimental instance works *with* the type system.
- `PathPattern`, `Method`, `Path`, and the typed client are all derived from the
`Endpoints` interface (`packages/rest-typings/src/index.ts:48-120`). Keeping
experimental paths **out** of that interface keeps the stable client surface clean
and forces explicit opt-in for experimental ones.
- The deprecation framework already writes `x-deprecation-*` response headers
(`apps/meteor/app/lib/server/lib/deprecationWarningLogger.ts:13-19`). We mirror that
pattern for an `x-experimental` / `Warning` signal.
- Auth, permissions, rate limiting, CORS, AJV validation, and metrics all come from
`createApi` + the middleware chain in `startRestAPI` — experimental endpoints get
them for free.

---

## Commit constraint

**Each implementation step below ships as exactly one commit.** No step is split across
multiple commits, and no commit spans more than one step. This keeps the history
bisectable, makes each phase independently reviewable and revertable, and maps the PR
review 1:1 onto the plan.

- A step's commit must leave the tree in a compiling, lint-clean state (`yarn lint
--quiet` passes) — partial work is squashed before committing.
- Commit message subject names the step, e.g. `feat(api): add experimental API instance
(step 1)`.
- If a step turns out to require a prerequisite not in the plan, add it to that step's
single commit rather than introducing an out-of-band commit.

## Implementation steps

### Step 1 — Add the `experimental` API instance

**File:** `apps/meteor/app/api/server/api.ts`

1. In the `API` object literal (around line 69-77), add:
```ts
experimental: createApi({ version: 'experimental', useDefaultAuth: true }),
```
Place it between `v1` and `default`.
2. Add `experimental: APIClass<'/experimental'>;` to the `API` type annotation
(around line 42-68) so it is typed.
3. If any `settings.watch(...)` callbacks need to refresh experimental routes the way
they refresh `API.v1` (rate limiter reloads at lines 92-100, custom fields at
79-90), add the matching `API.experimental?.…` calls. Optional for v1 of this work.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

**Acceptance:** `API.experimental.get('ping', { ... }, handler)` compiles and serves at
`GET /api/experimental/ping`.

**Commit (1 of 5):** `feat(api): add experimental API instance`

### Step 2 — Mount it in the request pipeline

**File:** `apps/meteor/app/api/server/api.ts`, `startRestAPI` (lines 102-123)

1. Insert `.use(API.experimental.router)` into the chain, **before**
`.use(API.default.router)` (line 121). Order matters: `default` is the catch-all.
2. Extend the metrics middleware `basePathRegex` (line 107) so experimental traffic is
measured. Either broaden the regex to `^\/api\/(v1|experimental)\//` or add a second
`metricsMiddleware` block pointed at `API.experimental`. Metrics are the canary used
later to decide whether an endpoint is ready for promotion to `/v1`.

**Acceptance:** experimental requests appear in the REST API Prometheus metrics with a
distinguishable path/label.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated

**Commit (2 of 5):** `feat(api): mount experimental router and metrics`

### Step 3 — Runtime "unstable" signal (mirror deprecation headers)

**New file:** `apps/meteor/app/api/server/middlewares/experimental.ts` (or colocate with
existing middlewares under `apps/meteor/app/api/server/middlewares/`).

1. Write a middleware that sets, on every response from the experimental instance:
```
Warning: 299 - "experimental: endpoint is unstable and may change without notice"
x-experimental: true
```
Comment thread
KevLehman marked this conversation as resolved.
`Warning: 299` is the RFC 7234 "miscellaneous persistent warning" code; `x-experimental`
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
is the easy programmatic check. Model the header-writing on
`writeDeprecationHeader` in `deprecationWarningLogger.ts:13-19`.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
2. Register the middleware on `API.experimental` only (not on `v1`/`default`).

**Acceptance:** every `/api/experimental/*` response carries both headers; `/api/v1/*`
responses do not.

**Commit (3 of 5):** `feat(api): add experimental unstable-signal middleware`

### Step 4 — Separate, opt-in SDK typings

**File:** `packages/rest-typings/src/index.ts` (+ a new file for the declarations)

1. Create `packages/rest-typings/src/experimental/index.ts` (new folder) and declare:
```ts
export type ExperimentalEndpoints = {
'/experimental/<name>': {
GET: (params: ...) => ...;
};
// ...
};
```
Follow the existing per-resource endpoint style (e.g.
`packages/rest-typings/src/v1/channels/channels.ts`).
2. Export `ExperimentalEndpoints` from the package root, but **do NOT** add it to the
`interface Endpoints extends ...` union (`index.ts:48-93`). This keeps `PathPattern`,
`Method`, `Path`, and the stable typed client free of experimental paths.
3. Consumers who want typed experimental calls import `ExperimentalEndpoints`
explicitly.

**Acceptance:** `import type { Endpoints } from '@rocket.chat/rest-typings'` does NOT
include experimental paths; `import type { ExperimentalEndpoints }` does.

**Commit (4 of 5):** `feat(rest-typings): add opt-in ExperimentalEndpoints`

### Step 5 — Guardrails (because it is a general mechanism)

1. **CI/lint guard:** add a check (script or eslint rule) asserting no path key present
in `ExperimentalEndpoints` is also present in `Endpoints`. This catches accidental
"promotion by copy-paste" that would silently create a semver obligation.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
2. **Promotion path:** document that stabilizing an endpoint means copying it to `/v1`
(optionally keeping the experimental path forwarding for a transition window).
Removal needs no deprecation cycle — but log removals for courtesy.
3. **Docs/CONTRIBUTING note:** state the no-semver guarantee and how to add an
experimental endpoint, so the mechanism is discoverable.
4. **OpenAPI/doc generation:** decide deliberately whether generated API docs scan only
`Endpoints` (experimental endpoints hidden — probably desirable) or also
`ExperimentalEndpoints`.

**Commit (5 of 5):** `chore(api): add experimental guardrails and docs`

---

## Test checklist

- [ ] `GET /api/experimental/<name>` resolves and returns the `x-experimental` + `Warning` headers.
- [ ] `/api/v1/*` responses are unchanged (no experimental headers).
- [ ] Auth / permissions / rate limiting enforced on an experimental route exactly as on `/v1`.
- [ ] `Endpoints` type does not include experimental paths; `ExperimentalEndpoints` does.
- [ ] CI guard fails if a path appears in both unions.
- [ ] Experimental requests show up in REST API metrics.

## Files touched (summary)

| File | Change |
| ---- | ------ |
| `apps/meteor/app/api/server/api.ts` | Add `experimental` instance, type entry, mount in `startRestAPI`, metrics regex |
| `apps/meteor/app/api/server/middlewares/experimental.ts` (new) | `x-experimental` / `Warning` header middleware |
| `packages/rest-typings/src/experimental/index.ts` (new) | `ExperimentalEndpoints` type, NOT merged into `Endpoints` |
| `packages/rest-typings/src/index.ts` | Export `ExperimentalEndpoints` |
| CI/lint config | Guard: no path in both `Endpoints` and `ExperimentalEndpoints` |
| docs / CONTRIBUTING | Document the contract + promotion path |
Loading
Loading