fix: 400 instead of 500 null dev id

docs/sdk/billing-deduct.md

@@ -36,6 +36,7 @@ Authorization: Bearer <jwt>
 | Field | Type | Required | Description |
 |---|---|---|---|
 | `requestId` | `string` | **Yes** | Unique idempotency key for this billing event. Must be a non-empty string. Reusing the same value returns the existing result with `alreadyProcessed: true`. |
+| `developerId` | `string` | No | The developer/account being billed. If omitted entirely, defaults to the authenticated user's ID. If provided, it must be a non-empty string — `null`, an empty string, or a non-string value are all rejected with `400 BAD_REQUEST` rather than being passed through to the billing service. |
 | `apiId` | `string` | **Yes** | The API being called. Non-empty string. |
 | `endpointId` | `string` | **Yes** | The specific endpoint being called. Non-empty string. |
 | `apiKeyId` | `string` | **Yes** | The API key used for the call. Non-empty string. |
@@ -133,6 +134,7 @@ the billing `requestId` body field.
 | Condition | HTTP | `code` | Message |
 |---|---|---|---|
 | Missing or empty `requestId` | 400 | `BAD_REQUEST` | `requestId is required and must be a non-empty string` |
+| `developerId` present but `null`, empty, or non-string | 400 | `BAD_REQUEST` | `developerId is required` |
 | Missing or empty `apiId` | 400 | `BAD_REQUEST` | `apiId is required and must be a non-empty string` |
 | Missing or empty `endpointId` | 400 | `BAD_REQUEST` | `endpointId is required and must be a non-empty string` |
 | Missing or empty `apiKeyId` | 400 | `BAD_REQUEST` | `apiKeyId is required and must be a non-empty string` |

src/middleware/errorHandler.ts

@@ -15,9 +15,6 @@ export interface ErrorResponseBody {
   details?: ValidationErrorDetail[];
 }
 
-import { errorEnvelope } from '../lib/envelope.js';
-import type { ErrorEnvelope } from '../types/ResponseEnvelope.js';
-
 function extractValidationDetails(err: unknown): ValidationErrorDetail[] | undefined {
   if (err instanceof ValidationError) {
     return err.details;
@@ -107,7 +104,7 @@ export function errorHandler(
   }
 
   const details = extractValidationDetails(err);
-  const body = buildErrorEnvelope(code, finalMessage, requestId, details);
+  const body = errorEnvelope(code, finalMessage, requestId, details);
 
   if (!res.headersSent) {
     res.status(statusCode).json(body);

src/routes/billing/deduct.test.ts

@@ -0,0 +1,108 @@
+import express from 'express';
+import request from 'supertest';
+import { errorHandler } from '../../middleware/errorHandler.js';
+import deductRouter from './deduct.js';
+import type { Pool } from 'pg';
+
+jest.mock('better-sqlite3', () => {
+  return class MockDatabase {
+    prepare() {
+      return { get: () => null };
+    }
+    exec() {
+      return undefined;
+    }
+    close() {
+      return undefined;
+    }
+  };
+});
+
+jest.mock('../../services/sorobanBilling.js', () => {
+  const actual = jest.requireActual('../../services/sorobanBilling.js');
+  return {
+    ...actual,
+    createSorobanRpcBillingClient: jest.fn().mockReturnValue({
+      getBalance: jest.fn(),
+      deductBalance: jest.fn(),
+    }),
+  };
+});
+
+describe('POST /api/billing/deduct - developerId validation', () => {
+  function buildApp(pool: Pool | null = { query: jest.fn() } as unknown as Pool) {
+    const app = express();
+    app.use(express.json());
+    if (pool) {
+      app.locals.dbPool = pool;
+    }
+    app.use('/api/billing/deduct', deductRouter);
+    app.use(errorHandler);
+    return app;
+  }
+
+  const validPayload = {
+    requestId: 'req_1',
+    apiId: 'api_1',
+    endpointId: 'endpoint_1',
+    apiKeyId: 'key_1',
+    amountUsdc: '0.01',
+  };
+
+  it('returns 400 (not 500) when developerId is explicitly null', async () => {
+    const res = await request(buildApp())
+      .post('/api/billing/deduct')
+      .set('x-user-id', 'user_123')
+      .send({ ...validPayload, developerId: null });
+
+    expect(res.status).toBe(400);
+    expect(res.body.success).toBe(false);
+    expect(res.body.error.code).toBe('BAD_REQUEST');
+    expect(res.body.error.message).toContain('developerId is required');
+  });
+
+  it('returns 400 when developerId is an empty string', async () => {
+    const res = await request(buildApp())
+      .post('/api/billing/deduct')
+      .set('x-user-id', 'user_123')
+      .send({ ...validPayload, developerId: '' });
+
+    expect(res.status).toBe(400);
+    expect(res.body.success).toBe(false);
+    expect(res.body.error.code).toBe('BAD_REQUEST');
+    expect(res.body.error.message).toContain('developerId is required');
+  });
+
+  it('returns 400 when developerId is not a string', async () => {
+    const res = await request(buildApp())
+      .post('/api/billing/deduct')
+      .set('x-user-id', 'user_123')
+      .send({ ...validPayload, developerId: 12345 });
+
+    expect(res.status).toBe(400);
+    expect(res.body.success).toBe(false);
+    expect(res.body.error.code).toBe('BAD_REQUEST');
+    expect(res.body.error.message).toContain('developerId is required');
+  });
+
+  it('falls back to the authenticated user id when developerId is omitted', async () => {
+    const queryMock = jest.fn().mockRejectedValue(new Error('stop before DB write'));
+    const res = await request(buildApp({ query: queryMock } as unknown as Pool))
+      .post('/api/billing/deduct')
+      .set('x-user-id', 'user_123')
+      .send(validPayload);
+
+    // Validation passes and the request proceeds past developerId handling
+    // (fails later at the DB layer, which is expected given the mocked pool).
+    expect(res.status).not.toBe(400);
+    expect(queryMock).toHaveBeenCalled();
+  });
+
+  it('returns 401 without auth', async () => {
+    const res = await request(buildApp())
+      .post('/api/billing/deduct')
+      .send({ ...validPayload, developerId: null });
+
+    expect(res.status).toBe(401);
+  });
+});

src/routes/billing/deduct.ts

@@ -84,6 +84,18 @@ function getPool(req: Request): Pool {
   return pool;
 }
 
+// idempotencyMiddleware declares an optional 4th `opts` parameter, giving it
+// an arity of 4. Express treats any 4-arg middleware function as an
+// error handler (function(err, req, res, next)), so registering it directly
+// causes thrown errors (e.g. validation errors) to be misrouted into it
+// instead of the real error handler, surfacing as 500s. Wrap it to a 3-arg
+// function so Express dispatches it as normal middleware.
+const idempotencyHandler = (
+  req: Request,
+  res: Response<unknown, AuthenticatedLocals>,
+  next: NextFunction,
+) => idempotencyMiddleware(req, res, next);
+
 function sendSimulationFailure(
   res: Response,
   result: Pick<BillingDeductResult, "error" | "simulationDetails">,
@@ -99,7 +111,7 @@ function sendSimulationFailure(
 router.post(
   "/",
   requireAuth,
-  idempotencyMiddleware,
+  idempotencyHandler,
   billingDeductHistogramMiddleware,
   async (
     req: Request,