mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-28 04:02:18 +01:00
## Thinking Path > - Paperclip validates API request bodies with Zod and converts validation failures into client errors. > - The global error handler recognized Zod failures with `instanceof ZodError`. > - Monorepo dependency layouts can provide more than one installed Zod module instance. > - A valid Zod error from another instance fails that identity check and falls through as HTTP 500. > - This pull request keeps the native path and adds a narrow structural fallback for named Zod errors with an issues array. > - The benefit is stable HTTP 400 validation semantics regardless of package-instance identity. ## Linked Issues or Issue Description Related but not duplicate: Refs #6908. That PR catches `instanceof ZodError` inside validation middleware and returns 422; it does not cover errors created by a second Zod module instance, which is the reproduced failure here. **What happened?** An invalid `POST /api/issues/:id/work-products` payload raised a real Zod validation error but returned HTTP 500 because the error came from a different Zod package instance. **Expected behavior** All genuine Zod validation failures return HTTP 400 with validation details, independent of module identity. **Steps to reproduce** 1. Submit a work-product body missing the required `provider`, `externalId`, and `url` fields. 2. Ensure the route schema is resolved from a different installed Zod instance than the server error handler. 3. Observe HTTP 500 before this fix. 4. Observe HTTP 400 after this fix. **Environment** - Paperclip base: `14f20be92b86a49ff2c35495e5b0fa4d719998ef` - Deployment: self-hosted, built from source - Access context: board API - Adapter scope: not adapter-specific - [x] I searched open PRs for `ZodError`, validation errors, and work-product validation and linked related work above. ## What Changed - Add a narrow `readZodIssues` helper that accepts native Zod errors or structurally valid cross-package Zod errors. - Preserve existing HTTP 400 response shape and structured error context. - Add a regression for a Zod error object from another module instance. ## Verification - `pnpm exec vitest run server/src/__tests__/error-handler.test.ts` - `pnpm --filter @paperclipai/server typecheck` - Full upstream CI test/build/e2e matrix passed. - Local post-deploy smoke returned HTTP 400 for the previously failing invalid work-product payload. ## Risks - A deliberately thrown object named `ZodError` with an `issues` array will be treated as a client validation failure. The effect is limited to returning HTTP 400 instead of 500; no authorization or persistence behavior changes. - No schema or migration changes. > This is a bug fix, not roadmap feature work. ## Model Used OpenAI Codex `gpt-5.6-sol`, with tool use, code execution, repository inspection, and independent read-only review agents. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either linked related public work and described the bug in-PR following the bug template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal ticket id - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have considered documentation; no user-facing documentation change is required - [x] I have considered and documented risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: cucurigoo <cucurigoo@users.noreply.github.com>
157 lines
5.2 KiB
TypeScript
157 lines
5.2 KiB
TypeScript
import type { Request, Response, NextFunction } from "express";
|
|
import type { Db } from "@paperclipai/db";
|
|
import { ZodError } from "zod";
|
|
import { HttpError } from "../errors.js";
|
|
import { trackErrorHandlerCrash } from "@paperclipai/shared/telemetry";
|
|
import { getTelemetryClient } from "../telemetry.js";
|
|
import { COMPANY_IMPORT_API_PATH } from "../routes/company-import-paths.js";
|
|
import { logger } from "./logger.js";
|
|
import {
|
|
recordResponsibleUserDenialOnActiveRun,
|
|
} from "../services/responsible-user-denial-run-outcomes.js";
|
|
|
|
export interface ErrorContext {
|
|
error: { message: string; stack?: string; name?: string; details?: unknown; raw?: unknown };
|
|
method: string;
|
|
url: string;
|
|
reqBody?: unknown;
|
|
reqParams?: unknown;
|
|
reqQuery?: unknown;
|
|
}
|
|
|
|
function isRedactedSkillPolicyDenial(details: Record<string, unknown> | null) {
|
|
return details?.code === "skill_policy_denied";
|
|
}
|
|
|
|
function readZodIssues(err: unknown): unknown[] | null {
|
|
if (err instanceof ZodError) return err.issues;
|
|
if (!err || typeof err !== "object" || (err as { name?: unknown }).name !== "ZodError") return null;
|
|
const issues = (err as { issues?: unknown }).issues;
|
|
return Array.isArray(issues) ? issues : null;
|
|
}
|
|
|
|
function attachErrorContext(
|
|
req: Request,
|
|
res: Response,
|
|
payload: ErrorContext["error"],
|
|
rawError?: Error,
|
|
) {
|
|
(res as any).__errorContext = {
|
|
error: payload,
|
|
method: req.method,
|
|
url: req.originalUrl,
|
|
reqBody: req.body,
|
|
reqParams: req.params,
|
|
reqQuery: req.query,
|
|
} satisfies ErrorContext;
|
|
if (rawError) {
|
|
(res as any).err = rawError;
|
|
}
|
|
}
|
|
|
|
function getPaperclipDb(req: Request): Db | null {
|
|
const locals = req.app?.locals as { paperclipDb?: Db; db?: Db } | undefined;
|
|
return locals?.paperclipDb ?? locals?.db ?? null;
|
|
}
|
|
|
|
function recordResponsibleUserDenialFromHttpError(
|
|
req: Request,
|
|
details: Record<string, unknown> | null,
|
|
) {
|
|
if (req.actor?.type !== "agent") return;
|
|
const db = getPaperclipDb(req);
|
|
if (!db) return;
|
|
|
|
void recordResponsibleUserDenialOnActiveRun(db, {
|
|
runId: req.actor.runId ?? null,
|
|
agentId: req.actor.agentId ?? null,
|
|
companyId: req.actor.companyId ?? null,
|
|
code: details?.code,
|
|
}).catch((recordErr) => {
|
|
logger.warn(
|
|
{
|
|
err: recordErr,
|
|
runId: req.actor?.runId ?? null,
|
|
agentId: req.actor?.type === "agent" ? req.actor.agentId ?? null : null,
|
|
},
|
|
"failed to record responsible-user denial on heartbeat run",
|
|
);
|
|
});
|
|
}
|
|
|
|
export function errorHandler(
|
|
err: unknown,
|
|
req: Request,
|
|
res: Response,
|
|
_next: NextFunction,
|
|
) {
|
|
if (err instanceof HttpError) {
|
|
const details = err.details && typeof err.details === "object" && !Array.isArray(err.details)
|
|
? err.details as Record<string, unknown>
|
|
: null;
|
|
const redactedSkillPolicyDenial = isRedactedSkillPolicyDenial(details);
|
|
const structuredConnectionError = new Set([
|
|
"user_authorization_required",
|
|
"grant_revoked",
|
|
"needs_reauthorization",
|
|
"installation_required",
|
|
"connection_not_installed",
|
|
"subject_not_permitted",
|
|
]).has(typeof details?.code === "string" ? details.code : "");
|
|
recordResponsibleUserDenialFromHttpError(req, details);
|
|
if (err.status >= 500) {
|
|
attachErrorContext(
|
|
req,
|
|
res,
|
|
{ message: err.message, stack: err.stack, name: err.name, details: err.details },
|
|
err,
|
|
);
|
|
const tc = getTelemetryClient();
|
|
if (tc) trackErrorHandlerCrash(tc, { errorCode: err.name });
|
|
}
|
|
res.status(err.status).json({
|
|
error: err.message,
|
|
...(typeof details?.code === "string" ? { code: details.code } : {}),
|
|
...(redactedSkillPolicyDenial && typeof details?.reason === "string" ? { reason: details.reason } : {}),
|
|
...(typeof details?.remediation === "string" || (structuredConnectionError && details?.remediation && typeof details.remediation === "object")
|
|
? { remediation: details.remediation }
|
|
: {}),
|
|
...(structuredConnectionError && details?.connection ? { connection: details.connection } : {}),
|
|
...(structuredConnectionError && details?.subject ? { subject: details.subject } : {}),
|
|
...(structuredConnectionError && typeof details?.grantId === "string" ? { grantId: details.grantId } : {}),
|
|
...(!redactedSkillPolicyDenial && err.details ? { details: err.details } : {}),
|
|
});
|
|
return;
|
|
}
|
|
|
|
const zodIssues = readZodIssues(err);
|
|
if (zodIssues) {
|
|
res.status(400).json({ error: "Validation error", details: zodIssues });
|
|
return;
|
|
}
|
|
|
|
const rootError = err instanceof Error ? err : new Error(String(err));
|
|
attachErrorContext(
|
|
req,
|
|
res,
|
|
err instanceof Error
|
|
? { message: err.message, stack: err.stack, name: err.name }
|
|
: { message: String(err), raw: err, stack: rootError.stack, name: rootError.name },
|
|
rootError,
|
|
);
|
|
|
|
const tc = getTelemetryClient();
|
|
if (tc) trackErrorHandlerCrash(tc, { errorCode: rootError.name });
|
|
|
|
res.status(500).json({
|
|
error: "Internal server error",
|
|
...(shouldExposeTrustedCloudTenantImportError(req) ? { message: rootError.message } : {}),
|
|
});
|
|
}
|
|
|
|
function shouldExposeTrustedCloudTenantImportError(req: Request) {
|
|
return req.actor?.source === "cloud_tenant"
|
|
&& req.method === "POST"
|
|
&& req.originalUrl.split("?")[0] === COMPANY_IMPORT_API_PATH;
|
|
}
|