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
49 changes: 31 additions & 18 deletions src/actions/portal/toc/new-toc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {
import { withDirPath } from '../../../infrastructure/tmp-extensions.js';
import { TempContext } from '../../../types/temp-context.js';
import { PortalService } from '../../../infrastructure/services/portal-service.js';
import { err, ok, Result } from 'neverthrow';
import { ServiceError } from '../../../infrastructure/service-error.js';

export class ContentContext {
private readonly fileService = new FileService();
Expand Down Expand Up @@ -52,6 +54,17 @@ export class PortalNewTocAction {
this.prompts.invalidBuildDirectory(buildDirectory);
return ActionResult.failed();
}

const specDirectory = buildDirectory.join('spec');
if (!(await this.fileService.directoryExists(specDirectory))) {
this.prompts.specDirectoryNotFound(buildDirectory);
return ActionResult.failed();
}
if (!(await buildContext.getSpecContext().validate())) {
this.prompts.specDirectoryEmpty(buildDirectory);
return ActionResult.failed();
}

const buildConfig = await buildContext.getBuildFileContents();
const contentDirectory = buildDirectory.join(buildConfig.contentFolder());

Expand All @@ -63,34 +76,34 @@ export class PortalNewTocAction {
return ActionResult.cancelled();
}

const tocComponents: TocComponents = await (async () => {
const specDirectory = buildDirectory.join('spec');

if (!(await this.fileService.directoryExists(specDirectory))) {
this.prompts.fallingBackToDefault();
return TocComponents.empty();
}

return await withDirPath(async (tempDirectory) => {
const tempContext = new TempContext(tempDirectory);
const specZipPath = await tempContext.zip(specDirectory);
const specFileStream = await this.fileService.getStream(specZipPath);
const tocComponentsResult: Result<TocComponents, ServiceError> = await withDirPath(async (tempDirectory) => {
const tempContext = new TempContext(tempDirectory);
const specZipPath = await tempContext.zip(specDirectory);
const specFileStream = await this.fileService.getStream(specZipPath);
try {
const result = await this.prompts.extractTocData(
this.portalService.generateTocData(specFileStream, this.configDirectory, this.commandMetadata),
expandEndpoints,
expandModels,
expandWebhooks,
expandCallbacks
);
specFileStream.close();
if (result.isErr()) {
this.prompts.fallingBackToDefault();
return TocComponents.empty();
return err(result.error);
}

return TocComponents.fromTocData(result.value);
});
})();
return ok(TocComponents.fromTocData(result.value));
} finally {
specFileStream.close();
}
});

if (tocComponentsResult.isErr()) {
this.prompts.tocExtractionFailed(tocComponentsResult.error.errorMessage);
return ActionResult.failed();
}
const tocComponents = tocComponentsResult.value;

const contentContext = new ContentContext(contentDirectory);
const contentExists = await contentContext.exists();

Expand Down
11 changes: 10 additions & 1 deletion src/infrastructure/service-error.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import axios from "axios";
import { ApiError, ProblemDetailsError } from "@apimatic/sdk";
import { format as f } from "../prompts/format.js";
import { discardStreamBody } from "../utils/utils.js";

export enum ServiceErrorCode {
NotFound = "NOT_FOUND",
Expand Down Expand Up @@ -97,7 +98,15 @@ function mapApiError(error: ApiError): ServiceError {
}

export function handleServiceError(error: unknown): ServiceError {
if (error instanceof ApiError) return mapApiError(error);
if (error instanceof ApiError) {
// A `callAsStream` error body would otherwise hang the CLI. Order against
// `mapApiError` is free: `error.result` is only populated when the SDK
// drained the body itself, so a live stream and a readable `result` never
// coexist.
const serviceError = mapApiError(error);
discardStreamBody(error.body);
return serviceError;
}

if (axios.isAxiosError(error)) {
const status = error.response?.status;
Expand Down
40 changes: 28 additions & 12 deletions src/infrastructure/services/transformation-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { FilePath } from "../../types/file/filePath.js";
import { CommandMetadata } from "../../types/common/command-metadata.js";
import { err, ok, Result} from "neverthrow";
import { ServiceError } from "../service-error.js";
import { discardStreamBody } from "../../utils/utils.js";

export interface TransformViaFileParams {
file: FilePath;
Expand Down Expand Up @@ -65,7 +66,7 @@ export class TransformationService {
apiValidationSummary
});
} catch (error) {
return err(await this.handleTransformationErrors(error));
return err(this.handleTransformationErrors(error));
}
}

Expand All @@ -80,18 +81,33 @@ export class TransformationService {
};
};

private readonly handleTransformationErrors = async (error: unknown): Promise<string> => {
if (error instanceof ApiError) {
const apiError = error as ApiError;
if (apiError.statusCode === 400) {
return "Your API Definition is invalid. Please use the APIMatic VS Code Extension to fix the errors and try again.";
} else if (apiError.statusCode === 401) {
const message = JSON.parse(apiError.body as string).message;
return ServiceError.unauthorizedWithHint(message).errorMessage;
}
return `Error ${apiError.statusCode}: An error occurred during the transformation. Please try again or contact support@apimatic.io for assistance.`;
} else {
private parseApiMessage(body: unknown): string | null {
if (typeof body !== "string") return null;
try {
return (JSON.parse(body) as { message?: string })?.message ?? null;
} catch {
return null;
}
}

private readonly handleTransformationErrors = (error: unknown): string => {
if (!(error instanceof ApiError)) {
return "An unexpected error occurred while validating your API Definition. Please try again later. If the problem persists, please reach out to our team at support@apimatic.io";
}

// `downloadTransformedFile` is a stream endpoint, so its error body is an
// undrained response that would keep the event loop alive. Discarding it
// first is safe: a string body (from `transformViaFile`) has no `destroy`
// and is left intact for `parseApiMessage` below.
discardStreamBody(error.body);

switch (error.statusCode) {
case 400:
return "Your API Definition is invalid. Please use the APIMatic VS Code Extension to fix the errors and try again.";
case 401:
return ServiceError.unauthorizedWithHint(this.parseApiMessage(error.body)).errorMessage;
default:
return `Error ${error.statusCode}: An error occurred during the transformation. Please try again or contact support@apimatic.io for assistance.`;
}
};
}
13 changes: 10 additions & 3 deletions src/prompts/portal/toc/new-toc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,22 @@ export class PortalNewTocPrompts {
return overwrite;
}

public fallingBackToDefault() {
log.warn(`Falling back to the default TOC structure.`);
public specDirectoryNotFound(directory: DirectoryPath) {
log.error(
`The ${f.var("spec")} directory was not found in ${f.path(directory)}.\n` +
`Add your API specification there and try again.`
);
}

public specDirectoryEmpty(directory: DirectoryPath) {
log.error(`The ${f.var("spec")} directory in ${f.path(directory)} is either empty or invalid.`);
}

public tocFileAlreadyExists() {
log.error(`Please enter a different destination path or delete the existing toc.yml file and try again.`);
}

public logError(message: string) {
public tocExtractionFailed(message: string) {
log.error(message);
}

Expand Down
4 changes: 0 additions & 4 deletions src/types/toc/toc-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,6 @@ export class TocComponents {
this.callbackGroups = callbackGroups;
}

static empty(): TocComponents {
return new TocComponents(new Map(), [], [], [], [], [], new Map(), new Map());
}

static fromTocData(tocData: TocData): TocComponents {
return new TocComponents(
TocComponents.toEndpointGroups(tocData.endpoints),
Expand Down
24 changes: 24 additions & 0 deletions src/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,30 @@ export async function parseStreamBodyToJson(body: NodeJS.ReadableStream): Promis
return JSON.parse(text);
}

// `destroy()` is not on `NodeJS.ReadableStream` — it lives on `stream.Readable`
// and `IncomingMessage` — so narrowing an `ApiError.body` union still won't let
// us call it. Probe for the method instead.
function isDestroyable(value: unknown): value is { destroy: () => void } {
return typeof (value as { destroy?: unknown })?.destroy === "function";
}

// Counterpart to `parseStreamBodyToJson`: releases a response body we are never
// going to read. An `ApiError.body` from a `callAsStream` endpoint is an
// undrained `IncomingMessage` whose socket keeps the Node event loop alive, so
// the CLI hangs after printing its outro — `outro()` only sets `process.exitCode`
// and we never call `process.exit()`.
//
// The axios adapter yields a stream on Node and a Blob elsewhere, so the probe
// above is a real platform branch. `destroy()` is idempotent, making this safe
// on bodies the SDK has already drained via `loadResult`. Callers that need the
// body must read it *before* calling this (see `PortalService.generatePortal`,
// which returns the 422 body as the error report).
export function discardStreamBody(body: unknown): void {
if (isDestroyable(body)) {
body.destroy();
}
}

export const toPascalCase = (str: string): string => {
return str
.split(" ")
Expand Down
69 changes: 69 additions & 0 deletions test/actions/portal/toc/new-toc.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import * as path from "path";
import fsExtra from "fs-extra";
import { expect } from "chai";
import { dir as tmpDir, DirectoryResult } from "tmp-promise";
import { PortalNewTocAction } from "../../../../src/actions/portal/toc/new-toc.js";
import { DirectoryPath } from "../../../../src/types/file/directoryPath.js";
import { CommandMetadata } from "../../../../src/types/common/command-metadata.js";

const COMMAND_METADATA: CommandMetadata = { commandName: "portal toc new", shell: "test" };

// `portal toc new` used to fall back to the default TOC when it had no spec to
// extract from — writing a toc.yml with no Endpoints, Events or Models sections
// over a possibly valid file, and exiting 0. The spec is now required. These
// cases return before any request is made, so no service stubbing is needed.
describe("PortalNewTocAction spec directory requirement", () => {
let tmpDirResult: DirectoryResult;
let buildDirectory: string;
let action: PortalNewTocAction;

const execute = () => action.execute(new DirectoryPath(buildDirectory));

beforeEach(async () => {
tmpDirResult = await tmpDir({ unsafeCleanup: true });
buildDirectory = path.join(tmpDirResult.path, "build");
await fsExtra.ensureDir(buildDirectory);
// `BuildContext.validate()` only checks that the build file exists — the
// spec checks run before its contents are ever read.
await fsExtra.writeJson(path.join(buildDirectory, "APIMATIC-BUILD.json"), {});

action = new PortalNewTocAction(new DirectoryPath(tmpDirResult.path), COMMAND_METADATA);
});

afterEach(async () => {
await tmpDirResult.cleanup();
});

it("fails when the spec directory is missing", async () => {
const result = await execute();

expect(result.isFailed()).to.be.true;
});

it("fails when the spec directory exists but is empty", async () => {
await fsExtra.ensureDir(path.join(buildDirectory, "spec"));

const result = await execute();

expect(result.isFailed()).to.be.true;
});

it("fails when the spec directory holds only dotfiles", async () => {
// `FileService.directoryEmpty` filters dot-prefixed entries, so a spec
// directory kept alive by a .gitkeep still has nothing to extract.
const specDirectory = path.join(buildDirectory, "spec");
await fsExtra.ensureDir(specDirectory);
await fsExtra.writeFile(path.join(specDirectory, ".gitkeep"), "");

const result = await execute();

expect(result.isFailed()).to.be.true;
});

it("writes no toc.yml when the spec is missing", async () => {
await execute();

const written = await fsExtra.readdir(buildDirectory);
expect(written).to.deep.equal(["APIMATIC-BUILD.json"]);
});
});
51 changes: 51 additions & 0 deletions test/utils/utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { Readable } from "stream";
import { expect } from "chai";
import { discardStreamBody } from "../../src/utils/utils.js";

// `ApiError.body` is `string | Blob | NodeJS.ReadableStream`, and which one it
// is depends on the endpoint: `callAsStream` yields a stream on Node, a Blob
// elsewhere, and `callAsJson` yields a string. `discardStreamBody` has to
// release the first without disturbing the others, so the probe is the contract.
describe("discardStreamBody", () => {
it("destroys a readable stream body", () => {
const stream = Readable.from(["chunk"]);
expect(stream.destroyed).to.be.false;

discardStreamBody(stream);

expect(stream.destroyed).to.be.true;
});

it("is idempotent on an already destroyed stream", () => {
// The SDK drains error bodies itself via `loadResult` on the `throwOn` path,
// so a body reaching here may already be finished.
const stream = Readable.from(["chunk"]);
stream.destroy();

expect(() => discardStreamBody(stream)).to.not.throw();
expect(stream.destroyed).to.be.true;
});

it("leaves a string body intact for callers that parse it afterwards", () => {
// `transformation-service` discards before reading the 401 message; that is
// only safe because a `callAsJson` string body has no `destroy`.
const body = JSON.stringify({ message: "Authorization has been denied" });

discardStreamBody(body);

expect((JSON.parse(body) as { message: string }).message).to.equal("Authorization has been denied");
});

it("ignores a Blob-like body with no destroy method", () => {
expect(() => discardStreamBody({ size: 12, type: "application/zip" })).to.not.throw();
});

it("ignores undefined and null bodies", () => {
expect(() => discardStreamBody(undefined)).to.not.throw();
expect(() => discardStreamBody(null)).to.not.throw();
});

it("ignores a destroy property that is not callable", () => {
expect(() => discardStreamBody({ destroy: "not a function" })).to.not.throw();
});
});
Loading