fix: ProblemDetails를 전송 계층이 실제로 만드는 모양 하나로 합친다
Task 3 리뷰 finding(Important): contract.ts의 손수 유지되는 ProblemDetails가 tech-log-studio-contract-contribution.ts의 envelopeError()가 실제로 반환하는 모양과 별도로 정의돼 있었다. envelopeError()는 ApiError의 type/title/status/detail/code/retryable/category/details만 채우므로 ProblemDetails가 갖고 있던 옛 평면 필드(instance/traceId/fieldErrors/ latestDocument/latestPublication/conflictingFields)는 production에서 항상 undefined였다 — mock만 채워서 mock이 아무것도 검증하지 못하는 상태였다. - contract.ts: ProblemDetails에서 옛 평면 필드를 제거하고 details를 wire와 같은 union 타입(ValidationErrorDetails | VersionConflictDetails | PublicationConflictDetails | null)으로 정확히 준다. 세 타입을 이제 개별 export한다. - tech-log-studio-contract-contribution.ts: envelopeError()가 contract.ts의 ProblemDetails를 그대로 반환 타입으로 쓴다(로컬 StudioProblemShape 제거) — 이제 한 곳에만 정의가 있다. - mock-studio-gateway.ts / cursor.ts: fieldErrors/latestDocument/ conflictingFields/latestPublication을 wire와 같은 자리(details 안)로 옮긴다. - mock-studio-gateway.test.ts: 위 이동에 맞춰 details를 캐스트로 좁혀 읽도록 갱신. retryable은 optional로 유지했다 — 여러 테스트가 생략하고 만들며, 이번 finding과 무관해 required로 좁히면 관련 없는 파일들이 깨진다. 리뷰가 보류한 2건(asset-upload-transport.ts의 CODES.has 중복 검사, apiErrorSchema.category가 enum이 아닌 것)은 손대지 않았다. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
DongHyeonka
co-authored by
Claude Opus 5
Claude Opus 5
parent
25a6b63d27
commit
d23a18f659
@@ -9,7 +9,9 @@ function invalid(detail: string) {
|
||||
const problem: ProblemDetails = {
|
||||
type: "https://techlog.local/problems/request-validation-failed", title: "Request validation failed",
|
||||
status: 422, detail, code: "REQUEST_VALIDATION_FAILED", retryable: false,
|
||||
fieldErrors: [{ path: "/cursor", message: detail }],
|
||||
// wire와 같은 자리: `details`가 `ValidationErrorDetails` 모양이다 (Task 3
|
||||
// fix round 1 — 예전엔 `fieldErrors`가 최상위 필드였다).
|
||||
details: { fieldErrors: [{ path: "/cursor", message: detail }] },
|
||||
};
|
||||
return new StudioGatewayError(problem);
|
||||
}
|
||||
|
||||
@@ -60,7 +60,11 @@ function gatewayProblem(status: number, code: ProblemDetails["code"], detail: st
|
||||
}
|
||||
|
||||
function requestError(fieldErrors: components["schemas"]["FieldError"][]) {
|
||||
return gatewayProblem(422, "REQUEST_VALIDATION_FAILED", "Request fields are invalid.", { fieldErrors });
|
||||
// wire와 같은 자리: `details`가 `ValidationErrorDetails` 모양이다 (Task 3
|
||||
// fix round 1 — 예전엔 `fieldErrors`가 최상위 필드였다).
|
||||
return gatewayProblem(422, "REQUEST_VALIDATION_FAILED", "Request fields are invalid.", {
|
||||
details: { fieldErrors },
|
||||
});
|
||||
}
|
||||
|
||||
function inputOf(document: WorkingCopy): WorkingCopyInput {
|
||||
@@ -169,7 +173,7 @@ export function createMockStudioGateway(supplied: Partial<MockStudioDependencies
|
||||
const nextAction = deriveDocumentState({ document: base.document, validation: base.currentValidation, preview: base.latestPreview, publication: base.currentPublication, dependencyRevision: base.dependencyRevision, now: dependencies.clock.now() }).nextAction;
|
||||
return { ...base, nextAction };
|
||||
};
|
||||
const version = (value: WorkingCopy, expected: number) => { if (value.version !== expected) throw gatewayProblem(409, "VERSION_CONFLICT", `Expected ${expected}; current ${value.version}.`, { latestDocument: clone(detail(value.id)), conflictingFields: [] }); };
|
||||
const version = (value: WorkingCopy, expected: number) => { if (value.version !== expected) throw gatewayProblem(409, "VERSION_CONFLICT", `Expected ${expected}; current ${value.version}.`, { details: { latestDocument: clone(detail(value.id)), conflictingFields: [] } }); };
|
||||
const structure = (input: WorkingCopyInput) => { const errors = validateWorkingCopyInputStructure(input); if (errors.length) throw requestError(errors); };
|
||||
const materialize = (id: string, value: number, input: WorkingCopyInput): WorkingCopy => ({ ...clone(input), id, version: value, updatedAt: dependencies.clock.now().toISOString(), relations: input.relations.map((relation) => ({ ...relation, id: relation.id ?? dependencies.idGenerator.next(), targetId: relation.targetId! })) }) as WorkingCopy;
|
||||
const summary = (value: WorkingCopy): components["schemas"]["DocumentSummary"] => {
|
||||
@@ -197,7 +201,7 @@ export function createMockStudioGateway(supplied: Partial<MockStudioDependencies
|
||||
getDocument(documentId, options) { return read(options, () => { uuid(documentId, "/documentId"); return detail(documentId); }); },
|
||||
saveDocument(documentId, command, options) { return idempotent("save", documentId, () => command, options, () => {
|
||||
uuid(documentId, "/documentId"); structure(command.document); const current = document(documentId); version(current, command.expectedVersion);
|
||||
if (state.pendingConflicts.delete(documentId)) { const latest = materialize(documentId, current.version + 1, { ...inputOf(current), title: "서버에서 먼저 수정된 제목", summary: "서버 최신 요약" }); state.documents.set(documentId, latest); state.validations.delete(documentId); throw gatewayProblem(409, "VERSION_CONFLICT", "The server document changed.", { latestDocument: clone(detail(documentId)), conflictingFields: ["/title", "/summary"] }); }
|
||||
if (state.pendingConflicts.delete(documentId)) { const latest = materialize(documentId, current.version + 1, { ...inputOf(current), title: "서버에서 먼저 수정된 제목", summary: "서버 최신 요약" }); state.documents.set(documentId, latest); state.validations.delete(documentId); throw gatewayProblem(409, "VERSION_CONFLICT", "The server document changed.", { details: { latestDocument: clone(detail(documentId)), conflictingFields: ["/title", "/summary"] } }); }
|
||||
const saved = materialize(documentId, current.version + 1, command.document); state.documents.set(documentId, saved); state.validations.delete(documentId); return detail(documentId);
|
||||
}); },
|
||||
validateDocument(documentId, command, options) { return idempotent("validate", documentId, () => command, options, () => { uuid(documentId, "/documentId"); const value = document(documentId); version(value, command.expectedVersion); const report = validateWorkingCopy(value, { now: dependencies.clock.now(), validationId: dependencies.idGenerator.next(), dependencyRevision: dependencies.dependencyRevision.current(), catalog: state.catalog, documents: [...state.documents.values()], assets: [...dependencies.assets.values()] }); state.validations.set(documentId, report); return report; }); },
|
||||
@@ -215,7 +219,7 @@ export function createMockStudioGateway(supplied: Partial<MockStudioDependencies
|
||||
const warnings = validation.issues.filter((issue) => issue.severity === "WARNING").map((issue) => issue.code); if (!exactSet(warnings, command.acknowledgedWarningCodes)) throw requestError([{ path: "/acknowledgedWarningCodes", message: "Acknowledge all current warnings." }]);
|
||||
const eventId = dependencies.idGenerator.next(); const publicationId = existing?.publicationId ?? dependencies.idGenerator.next(); const event: PublicationEvent = { publicationEventId: eventId, publicationId, documentId, type: existing ? "REPUBLISHED" : "PUBLISHED", occurredAt: now.toISOString(), publishedVersion: value.version, sourcePublishedEventId: null, snapshotAvailable: true }; const publication: PublicationAggregate = { publicationId, documentId, status: "PUBLISHED", publishedVersion: value.version, publicationRevision: (existing?.publicationRevision ?? 0) + 1, latestEventId: eventId, publicPath: preview.renderModel.publicPath, updatedAt: now.toISOString() }; state.events.set(eventId, event); state.publications.set(documentId, publication); state.snapshots.set(eventId, { event: clone(event), renderModel: clone(preview.renderModel), contentFormatVersion: CONTENT_FORMAT_VERSION, rendererContractVersion: RENDERER_CONTRACT_VERSION }); return { publication, event } satisfies PublishResult;
|
||||
}); },
|
||||
unpublishPublication(publicationId, command, options) { return idempotent("unpublish", publicationId, () => command, options, () => { uuid(publicationId, "/publicationId"); const current = [...state.publications.values()].find((item) => item.publicationId === publicationId); if (!current) throw gatewayProblem(404, "PUBLICATION_NOT_FOUND", "Publication not found."); if (current.publicationRevision !== command.expectedPublicationRevision) throw gatewayProblem(409, "PUBLICATION_CONFLICT", "Publication revision changed.", { latestPublication: clone(current) }); if (current.status === "UNPUBLISHED") return { publication: current, event: state.events.get(current.latestEventId)! }; const event: PublicationEvent = { publicationEventId: dependencies.idGenerator.next(), publicationId, documentId: current.documentId, type: "UNPUBLISHED", occurredAt: dependencies.clock.now().toISOString(), publishedVersion: current.publishedVersion, sourcePublishedEventId: current.latestEventId, snapshotAvailable: false }; const publication: PublicationAggregate = { ...current, status: "UNPUBLISHED", publicationRevision: current.publicationRevision + 1, latestEventId: event.publicationEventId, updatedAt: event.occurredAt }; state.events.set(event.publicationEventId, event); state.publications.set(current.documentId, publication); return { publication, event }; }); },
|
||||
unpublishPublication(publicationId, command, options) { return idempotent("unpublish", publicationId, () => command, options, () => { uuid(publicationId, "/publicationId"); const current = [...state.publications.values()].find((item) => item.publicationId === publicationId); if (!current) throw gatewayProblem(404, "PUBLICATION_NOT_FOUND", "Publication not found."); if (current.publicationRevision !== command.expectedPublicationRevision) throw gatewayProblem(409, "PUBLICATION_CONFLICT", "Publication revision changed.", { details: { latestPublication: clone(current) } }); if (current.status === "UNPUBLISHED") return { publication: current, event: state.events.get(current.latestEventId)! }; const event: PublicationEvent = { publicationEventId: dependencies.idGenerator.next(), publicationId, documentId: current.documentId, type: "UNPUBLISHED", occurredAt: dependencies.clock.now().toISOString(), publishedVersion: current.publishedVersion, sourcePublishedEventId: current.latestEventId, snapshotAvailable: false }; const publication: PublicationAggregate = { ...current, status: "UNPUBLISHED", publicationRevision: current.publicationRevision + 1, latestEventId: event.publicationEventId, updatedAt: event.occurredAt }; state.events.set(event.publicationEventId, event); state.publications.set(current.documentId, publication); return { publication, event }; }); },
|
||||
listPublications(query, options) { return read(options, () => { queryText(query.q); const limit = limitOf(query.limit); const normalized = { q: normalizeQ(query.q), type: query.type ?? null, sort: "OCCURRED_DESC" }; const binding = cursorBinding(normalized); const cursor = query.cursor ? decodeCursor(query.cursor, binding) : null; const all = [...state.events.values()].filter((event) => (!normalized.type || event.type === normalized.type) && (!normalized.q || `${document(event.documentId).title} ${document(event.documentId).summary}`.toLocaleLowerCase("ko-KR").includes(normalized.q))).sort((a, b) => b.occurredAt.localeCompare(a.occurredAt) || a.publicationEventId.localeCompare(b.publicationEventId)); const source = cursor ? all.filter((event) => event.occurredAt < cursor.lastValue || (event.occurredAt === cursor.lastValue && event.publicationEventId > cursor.lastId)) : all; const selected = source.slice(0, limit); const last = selected.at(-1); return { items: selected.map(publicationRow), nextCursor: selected.length < source.length && last ? encodeCursor({ binding, lastValue: last.occurredAt, lastId: last.publicationEventId }) : null } satisfies PublicationPage; }); },
|
||||
getPublicationSnapshot(publicationEventId, options) { return read(options, () => { uuid(publicationEventId, "/publicationEventId"); if (!state.events.has(publicationEventId)) throw gatewayProblem(404, "PUBLICATION_EVENT_NOT_FOUND", "Publication event not found."); const snapshot = state.snapshots.get(publicationEventId); if (!snapshot) throw gatewayProblem(404, "PUBLICATION_SNAPSHOT_NOT_FOUND", "Publication snapshot not found."); return snapshot; }); },
|
||||
getCatalog(query, options) { return read(options, () => { if (!query.type) throw requestError([{ path: "/type", message: "type is required." }]); queryText(query.q); const limit = limitOf(query.limit); const normalized = { type: query.type, q: normalizeQ(query.q), sort: "LABEL_ASC" }; const binding = cursorBinding(normalized); const cursor = query.cursor ? decodeCursor(query.cursor, binding) : null; const all = state.catalog.filter((item) => item.type === query.type && (!normalized.q || item.label.toLocaleLowerCase("ko-KR").includes(normalized.q))).sort((a, b) => a.label.localeCompare(b.label, "ko") || a.id.localeCompare(b.id)); const source = cursor ? all.filter((item) => item.label.localeCompare(cursor.lastValue, "ko") > 0 || (item.label === cursor.lastValue && item.id > cursor.lastId)) : all; const selected = source.slice(0, limit); const last = selected.at(-1); return { items: selected, nextCursor: selected.length < source.length && last ? encodeCursor({ binding, lastValue: last.label, lastId: last.id }) : null } satisfies CatalogPage; }); },
|
||||
|
||||
@@ -25,14 +25,32 @@ export type PublicationSnapshot = Schemas["PublicationSnapshot"];
|
||||
export type CatalogPage = Schemas["CatalogPage"];
|
||||
/**
|
||||
* ADR-006으로 canonical 계약의 오류가 봉투(`ErrorEnvelope`/`ApiError`)로
|
||||
* 바뀌면서 `ProblemDetails` 스키마 자체는 canonical에서 삭제됐다. 앱 계층
|
||||
* (`StudioGatewayError`, mock 게이트웨이들, `asset-upload-transport.ts`)은
|
||||
* 여전히 이 평평한(flat) 모양을 소비한다 — 전송 경계
|
||||
* (`tech-log-studio-contract-contribution.ts`의 `envelopeError`)가 `ApiError`를
|
||||
* 여기로 옮기고, mock은 이 모양을 직접 구성한다. 그래서 더 이상 생성된
|
||||
* 스키마에서 뽑지 않고 여기서 손으로 유지한다.
|
||||
* 바뀌면서 `ProblemDetails` 스키마 자체는 canonical에서 삭제됐다. 더 이상
|
||||
* 생성된 스키마에서 뽑지 않고 여기서 손으로 유지하되, 이 모양은 전송 경계
|
||||
* (`tech-log-studio-contract-contribution.ts`의 `envelopeError`)가 실제로
|
||||
* 만드는 모양과 **동일해야 한다** — 그게 이 값이 production에서 채워지는
|
||||
* 유일한 경로다. `ApiError`가 옮겨주는 필드(`type/title/status/detail/code/
|
||||
* category/retryable/details`)만 갖는다.
|
||||
*
|
||||
* (Task 3 fix round 1) 이전에는 ADR-006 이전 평면 wire 모양에서 넘어온
|
||||
* `instance`/`traceId`/`fieldErrors`/`latestDocument`/`latestPublication`/
|
||||
* `conflictingFields`를 최상위 필드로 따로 두고 있었다. `envelopeError`는
|
||||
* 그 필드들을 채우지 않으므로(옮길 대상이 없음) production 값에서는 항상
|
||||
* `undefined`였고, mock 게이트웨이만 채웠다 — 타입은 있는데 mock에 대고
|
||||
* 짜면 통과하고 실제 HTTP 경로에서는 조용히 비는, 봉투 검증 설계가 막으려던
|
||||
* 함정이었다. 그 데이터는 이제 wire와 동일하게 `details` 안에 둔다 — mock도
|
||||
* 여기 채운다(`mock-studio-gateway.ts`, `cursor.ts`).
|
||||
*/
|
||||
export type ProblemDetails = {
|
||||
export type ValidationErrorDetails = Schemas["ValidationErrorDetails"];
|
||||
export type VersionConflictDetails = Schemas["VersionConflictDetails"];
|
||||
export type PublicationConflictDetails = Schemas["PublicationConflictDetails"];
|
||||
export type ProblemDetailsPayload =
|
||||
| ValidationErrorDetails
|
||||
| VersionConflictDetails
|
||||
| PublicationConflictDetails
|
||||
| null;
|
||||
|
||||
export type ProblemDetails = Readonly<{
|
||||
/** Format: uri-reference */
|
||||
type: string;
|
||||
title: string;
|
||||
@@ -40,18 +58,13 @@ export type ProblemDetails = {
|
||||
detail: string;
|
||||
code: Schemas["ApiError"]["code"];
|
||||
category?: Schemas["ApiError"]["category"];
|
||||
// optional 유지: 기존 호출부(테스트의 `new StudioGatewayError({...})` 리터럴
|
||||
// 다수, `synthetic()`의 일부 경로)가 `retryable`을 생략한다. 이번 fix
|
||||
// round의 finding은 `details`/평면 필드 문제이지 이 필드의 필수 여부가
|
||||
// 아니다 — required로 좁히면 무관한 파일들이 깨진다.
|
||||
retryable?: boolean;
|
||||
details?: unknown;
|
||||
/** Format: uri-reference */
|
||||
instance?: string;
|
||||
traceId?: string;
|
||||
fieldErrors?: Schemas["FieldError"][];
|
||||
latestDocument?: Schemas["WorkingCopyDetail"];
|
||||
latestPublication?: Schemas["PublicationAggregate"];
|
||||
conflictingFields?: string[];
|
||||
} & {
|
||||
[key: string]: unknown;
|
||||
};
|
||||
details?: ProblemDetailsPayload;
|
||||
}>;
|
||||
export type PublicRenderModel = Schemas["PublicRenderModel"];
|
||||
export type Asset = Schemas["Asset"];
|
||||
export type AssetDetail = Schemas["AssetDetail"];
|
||||
|
||||
@@ -8,6 +8,7 @@ import type {
|
||||
} from "../../../contracts/external-contract-runtime.ts";
|
||||
import { TECH_LOG_FEATURE_ID } from "../application/tech-log-feature-input.ts";
|
||||
import canonicalSource from "./studio/canonical-source.json" with { type: "json" };
|
||||
import type { ProblemDetails } from "./studio/contract.ts";
|
||||
import { STUDIO_ERROR_CODES } from "../adapters/http/studio-error-mapping.ts";
|
||||
import {
|
||||
assertExactlyOneTechLogStudioBootstrapOperation,
|
||||
@@ -84,9 +85,17 @@ const apiErrorSchema = z
|
||||
* 그 모양을 계속 쓰므로 매핑을 여기서 끝내면 아래 계층이 무변경이다.
|
||||
* `status`는 봉투에 없다 — 전송 계층이 실제 HTTP status를 따로 들고 있으므로
|
||||
* 0으로 두고 `toStudioGatewayError`가 outcome의 status로 덮는다.
|
||||
*
|
||||
* (Task 3 fix round 1) `ProblemDetails`는 `contract.ts`에서 가져온다 — 이
|
||||
* transform이 실제로 만드는 모양과 `contract.ts`가 선언하는 모양이 서로 다른
|
||||
* 파일에서 독립적으로 정의되면(원래 상태) 둘이 갈라져도 아무 게이트도 못
|
||||
* 잡는다. `details`는 wire 그대로 통째로 옮긴다 — `fieldErrors`/
|
||||
* `latestDocument`/`conflictingFields`/`latestPublication`으로 분해하지
|
||||
* 않는다. 지금 그 필드들을 평평하게 읽는 소비자가 없고, 분해는 실제 소비자가
|
||||
* 생겼을 때 추가할 투기적 작업이다.
|
||||
*/
|
||||
export const envelopeError = (): RuntimeValidator<StudioProblemShape> =>
|
||||
zodValidator<StudioProblemShape>(
|
||||
export const envelopeError = (): RuntimeValidator<ProblemDetails> =>
|
||||
zodValidator<ProblemDetails>(
|
||||
"StudioErrorEnvelope",
|
||||
z
|
||||
.object({ success: z.literal(false), error: apiErrorSchema, meta: metaSchema })
|
||||
@@ -99,25 +108,14 @@ export const envelopeError = (): RuntimeValidator<StudioProblemShape> =>
|
||||
code: envelope.error.code,
|
||||
retryable: envelope.error.retryable,
|
||||
category: envelope.error.category,
|
||||
details: (envelope.error as { details?: unknown }).details ?? null,
|
||||
})) as unknown as z.ZodType<StudioProblemShape>,
|
||||
details: (envelope.error as { details?: ProblemDetails["details"] }).details ?? null,
|
||||
})) as unknown as z.ZodType<ProblemDetails>,
|
||||
);
|
||||
|
||||
export type StudioProblemShape = Readonly<{
|
||||
type: string;
|
||||
title: string;
|
||||
status: number;
|
||||
detail: string;
|
||||
code: string;
|
||||
retryable: boolean;
|
||||
category: string;
|
||||
details: unknown;
|
||||
}>;
|
||||
|
||||
const PROBLEM = envelopeError();
|
||||
|
||||
/** 4xx 도메인 거절은 적용되지 않았음이 확정이다. 5xx/네트워크는 불확정이다. */
|
||||
const COMMAND_EFFECT: CommandEffectDescriptor<StudioProblemShape> =
|
||||
const COMMAND_EFFECT: CommandEffectDescriptor<ProblemDetails> =
|
||||
Object.freeze({
|
||||
successEffect: "APPLIED_CONFIRMED" as const,
|
||||
classifyProblem({ status }: Readonly<{ status: number; problem: unknown }>) {
|
||||
|
||||
@@ -3,6 +3,8 @@ import { test } from "vitest";
|
||||
|
||||
import { isStudioGatewayError } from "../../../src/features/tech-log/application/ports/studio-gateway-error.ts";
|
||||
import type {
|
||||
ValidationErrorDetails,
|
||||
VersionConflictDetails,
|
||||
WorkingCopy,
|
||||
WorkingCopyInput,
|
||||
} from "../../../src/features/tech-log/contracts/studio/contract.ts";
|
||||
@@ -71,8 +73,11 @@ function isProblem(status: number, code: string, paths?: string[]) {
|
||||
assert.equal(error.status, status);
|
||||
assert.equal(error.code, code);
|
||||
if (paths) {
|
||||
// wire와 같은 자리: `REQUEST_VALIDATION_FAILED`의 detail은
|
||||
// `ValidationErrorDetails`로 `details`에 있다 (Task 3 fix round 1).
|
||||
const details = error.problem.details as ValidationErrorDetails;
|
||||
assert.deepEqual(
|
||||
error.problem.fieldErrors?.map(({ path }) => path),
|
||||
details.fieldErrors.map(({ path }) => path),
|
||||
paths,
|
||||
);
|
||||
}
|
||||
@@ -203,9 +208,8 @@ test("runtime OpenAPI validation enforces discriminators, extras, dates, UUIDs a
|
||||
}),
|
||||
(error) => {
|
||||
assert.ok(isStudioGatewayError(error));
|
||||
assert.ok(
|
||||
error.problem.fieldErrors?.some(({ path }) => path === pointer),
|
||||
);
|
||||
const details = error.problem.details as ValidationErrorDetails;
|
||||
assert.ok(details.fieldErrors.some(({ path }) => path === pointer));
|
||||
return true;
|
||||
},
|
||||
);
|
||||
@@ -575,12 +579,10 @@ test("publication history, immutable snapshots, conflict fixtures and 404 bodies
|
||||
(error) => {
|
||||
assert.ok(isStudioGatewayError(error));
|
||||
assert.equal(error.code, "VERSION_CONFLICT");
|
||||
assert.deepEqual(error.problem.conflictingFields, [
|
||||
"/title",
|
||||
"/summary",
|
||||
]);
|
||||
const details = error.problem.details as VersionConflictDetails;
|
||||
assert.deepEqual(details.conflictingFields, ["/title", "/summary"]);
|
||||
assert.equal(
|
||||
error.problem.latestDocument?.document.version,
|
||||
details.latestDocument.document.version,
|
||||
conflict.document.version + 1,
|
||||
);
|
||||
return true;
|
||||
|
||||
Reference in New Issue
Block a user