Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standardize CLI Errors #6391

Merged
merged 10 commits into from
Jan 21, 2025
Merged

Standardize CLI Errors #6391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f34a3ec
Standardize CLI errors with codes
jdolle Jan 18, 2025
cc39f83
Add example to docs
jdolle Jan 18, 2025
f7597ba
Merge branch 'main' into cli-error-codes
jdolle Jan 18, 2025
bb72182
Fix imports
jdolle Jan 18, 2025
3ea07ec
Prettier write
jdolle Jan 18, 2025
d769236
Improve token and file reading errors
jdolle Jan 19, 2025
8d7ef3d
Prettier
jdolle Jan 19, 2025
06e158c
Match on clierror instead of exiterror for rethrow
jdolle Jan 21, 2025
8fdcb6d
Update snapshots
jdolle Jan 21, 2025
e97cde6
Move error code docs to a separate change
jdolle Jan 21, 2025
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
54 changes: 36 additions & 18 deletions integration-tests/tests/cli/__snapshots__/schema.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,13 @@ exports[`FEDERATION > publishing invalid schema SDL provides meaningful feedback
exitCode------------------------------------------:
2
stderr--------------------------------------------:
Error: The SDL is not valid at line 1, column 1:
Syntax Error: Unexpected Name "iliketurtles".
› Error: The SDL is not valid at line 1, column 1:
› Syntax Error: Unexpected Name "iliketurtles". [301]
› > See https://__URL__ for
› a complete list of error codes and recommended fixes.
› To disable this message set HIVE_NO_ERROR_TIP=1
stdout--------------------------------------------:
✖ Failed to publish schema
__NONE__
`;

exports[`FEDERATION > schema:check should notify user when registry is empty > schemaCheck 1`] = `
Expand Down Expand Up @@ -160,10 +163,13 @@ exports[`FEDERATION > schema:publish should see Invalid Token error when token i
exitCode------------------------------------------:
2
stderr--------------------------------------------:
› Error: Invalid token provided
› Reference: __ID__
› Error: A valid registry token is required to perform the action. The
› registry token used does not exist or has been revoked. [106]
› > See https://__URL__ for
› a complete list of error codes and recommended fixes.
› To disable this message set HIVE_NO_ERROR_TIP=1
stdout--------------------------------------------:
✖ Failed to publish schema
__NONE__
`;

exports[`SINGLE > can publish a schema with breaking, warning and safe changes > schemaCheck 1`] = `
Expand Down Expand Up @@ -246,10 +252,13 @@ exports[`SINGLE > publishing invalid schema SDL provides meaningful feedback for
exitCode------------------------------------------:
2
stderr--------------------------------------------:
Error: The SDL is not valid at line 1, column 1:
Syntax Error: Unexpected Name "iliketurtles".
› Error: The SDL is not valid at line 1, column 1:
› Syntax Error: Unexpected Name "iliketurtles". [301]
› > See https://__URL__ for
› a complete list of error codes and recommended fixes.
› To disable this message set HIVE_NO_ERROR_TIP=1
stdout--------------------------------------------:
✖ Failed to publish schema
__NONE__
`;

exports[`SINGLE > schema:check should notify user when registry is empty > schemaCheck 1`] = `
Expand Down Expand Up @@ -310,10 +319,13 @@ exports[`SINGLE > schema:publish should see Invalid Token error when token is in
exitCode------------------------------------------:
2
stderr--------------------------------------------:
› Error: Invalid token provided
› Reference: __ID__
› Error: A valid registry token is required to perform the action. The
› registry token used does not exist or has been revoked. [106]
› > See https://__URL__ for
› a complete list of error codes and recommended fixes.
› To disable this message set HIVE_NO_ERROR_TIP=1
stdout--------------------------------------------:
✖ Failed to publish schema
__NONE__
`;

exports[`STITCHING > can publish a schema with breaking, warning and safe changes > schemaCheck 1`] = `
Expand Down Expand Up @@ -412,10 +424,13 @@ exports[`STITCHING > publishing invalid schema SDL provides meaningful feedback
exitCode------------------------------------------:
2
stderr--------------------------------------------:
Error: The SDL is not valid at line 1, column 1:
Syntax Error: Unexpected Name "iliketurtles".
› Error: The SDL is not valid at line 1, column 1:
› Syntax Error: Unexpected Name "iliketurtles". [301]
› > See https://__URL__ for
› a complete list of error codes and recommended fixes.
› To disable this message set HIVE_NO_ERROR_TIP=1
stdout--------------------------------------------:
✖ Failed to publish schema
__NONE__
`;

exports[`STITCHING > schema:check should notify user when registry is empty > schemaCheck 1`] = `
Expand Down Expand Up @@ -478,8 +493,11 @@ exports[`STITCHING > schema:publish should see Invalid Token error when token is
exitCode------------------------------------------:
2
stderr--------------------------------------------:
› Error: Invalid token provided
› Reference: __ID__
› Error: A valid registry token is required to perform the action. The
› registry token used does not exist or has been revoked. [106]
› > See https://__URL__ for
› a complete list of error codes and recommended fixes.
› To disable this message set HIVE_NO_ERROR_TIP=1
stdout--------------------------------------------:
✖ Failed to publish schema
__NONE__
`;
220 changes: 118 additions & 102 deletions packages/libraries/cli/src/base-command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,21 @@
import { print, type GraphQLError } from 'graphql';
import { existsSync, readFileSync } from 'node:fs';
import { env } from 'node:process';
import { print } from 'graphql';
import type { ExecutionResult } from 'graphql';
import { http } from '@graphql-hive/core';
import type { TypedDocumentNode } from '@graphql-typed-document-node/core';
import { Command, Errors, Flags, Interfaces } from '@oclif/core';
import { Command, Flags, Interfaces } from '@oclif/core';
import { Config, GetConfigurationValueType, ValidConfigurationKeys } from './helpers/config';
import {
APIError,
FileMissingError,
HTTPError,
InvalidFileContentsError,
InvalidRegistryTokenError,
isAggregateError,
MissingArgumentsError,
NetworkError,
} from './helpers/errors';
import { Texture } from './helpers/texture/texture';

export type Flags<T extends typeof Command> = Interfaces.InferredFlags<
Expand Down Expand Up @@ -57,7 +69,7 @@ export default abstract class BaseCommand<T extends typeof Command> extends Comm
}

logFailure(...args: any[]) {
this.log(Texture.failure(...args));
this.logToStderr(Texture.failure(...args));
}

logInfo(...args: any[]) {
Expand Down Expand Up @@ -98,7 +110,7 @@ export default abstract class BaseCommand<T extends typeof Command> extends Comm
* @param key
* @param args all arguments or flags
* @param defaultValue default value
* @param message custom error message in case of no value
* @param description description of the flag in case of no value
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of a message as an argument, I standardized the message format and if a more custom message or suggested fix would be useful, then capture the error and throw the custom error.

* @param env an env var name
*/
ensure<
Expand All @@ -111,8 +123,8 @@ export default abstract class BaseCommand<T extends typeof Command> extends Comm
args,
legacyFlagName,
defaultValue,
message,
env,
env: envName,
description,
}: {
args: TArgs;
key: TKey;
Expand All @@ -127,38 +139,34 @@ export default abstract class BaseCommand<T extends typeof Command> extends Comm
}>;

defaultValue?: TArgs[keyof TArgs] | null;
message?: string;
description: string;
env?: string;
}): NonNullable<GetConfigurationValueType<TKey>> | never {
if (args[key] != null) {
return args[key] as NonNullable<GetConfigurationValueType<TKey>>;
}

if (legacyFlagName && (args as any)[legacyFlagName] != null) {
return args[legacyFlagName] as any as NonNullable<GetConfigurationValueType<TKey>>;
}

// eslint-disable-next-line no-process-env
if (env && process.env[env]) {
// eslint-disable-next-line no-process-env
return process.env[env] as TArgs[keyof TArgs] as NonNullable<GetConfigurationValueType<TKey>>;
}

const userConfigValue = this._userConfig!.get(key);

if (userConfigValue != null) {
return userConfigValue;
}
let value: GetConfigurationValueType<TKey>;

if (defaultValue) {
return defaultValue;
if (args[key] != null) {
value = args[key];
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are so that empty strings dont fallback to the default.
I believe empty strings for a required argument indicates there is something wrong, like a variable wasnt set within a script correctly

} else if (legacyFlagName && (args as any)[legacyFlagName] != null) {
value = args[legacyFlagName] as NonNullable<GetConfigurationValueType<TKey>>;
} else if (envName && env[envName] !== undefined) {
value = env[envName] as TArgs[keyof TArgs] as NonNullable<GetConfigurationValueType<TKey>>;
} else {
const configValue = this._userConfig!.get(key) as NonNullable<
GetConfigurationValueType<TKey>
>;

if (configValue !== undefined) {
value = configValue;
} else if (defaultValue) {
value = defaultValue;
}
}

if (message) {
throw new Errors.CLIError(message);
if (value?.length) {
return value;
}

throw new Errors.CLIError(`Missing "${String(key)}"`);
throw new MissingArgumentsError([String(key), description]);
}

cleanRequestId(requestId?: string | null) {
Expand Down Expand Up @@ -186,7 +194,7 @@ export default abstract class BaseCommand<T extends typeof Command> extends Comm
const isDebug = this.flags.debug;

return {
async request<TResult, TVariables>(
request: async <TResult, TVariables>(
args: {
operation: TypedDocumentNode<TResult, TVariables>;
/** timeout in milliseconds */
Expand All @@ -198,43 +206,72 @@ export default abstract class BaseCommand<T extends typeof Command> extends Comm
: {
variables: TVariables;
}),
): Promise<TResult> {
const response = await http.post(
endpoint,
JSON.stringify({
query: typeof args.operation === 'string' ? args.operation : print(args.operation),
variables: args.variables,
}),
{
logger: {
info: (...args) => {
if (isDebug) {
console.info(...args);
}
},
error: (...args) => {
console.error(...args);
): Promise<TResult> => {
let response: Response;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

converted function format to carry over the this

try {
response = await http.post(
endpoint,
JSON.stringify({
query: typeof args.operation === 'string' ? args.operation : print(args.operation),
variables: args.variables,
}),
{
logger: {
info: (...args) => {
if (isDebug) {
this.logInfo(...args);
}
},
error: (...args) => {
// Allow retrying requests without noise
if (isDebug) {
this.logWarning(...args);
}
},
},
headers: requestHeaders,
timeout: args.timeout,
},
headers: requestHeaders,
timeout: args.timeout,
},
);
);
} catch (e: any) {
const sourceError = e?.cause ?? e;
if (isAggregateError(sourceError)) {
throw new NetworkError(sourceError.errors[0]?.message);
} else {
throw new NetworkError(sourceError);
}
}

if (!response.ok) {
throw new Error(`Invalid status code for HTTP call: ${response.status}`);
throw new HTTPError(
endpoint,
response.status,
response.statusText ?? 'Invalid status code for HTTP call',
);
}

let jsonData;
try {
jsonData = (await response.json()) as ExecutionResult<TResult>;
} catch (err) {
const contentType = response?.headers?.get('content-type');
throw new APIError(
`Response from graphql was not valid JSON.${contentType ? ` Received "content-type": "${contentType}".` : ''}`,
this.cleanRequestId(response?.headers?.get('x-request-id')),
);
}
const jsonData = (await response.json()) as ExecutionResult<TResult>;

if (jsonData.errors && jsonData.errors.length > 0) {
throw new ClientError(
`Failed to execute GraphQL operation: ${jsonData.errors
.map(e => e.message)
.join('\n')}`,
{
errors: jsonData.errors,
headers: response.headers,
},
if (jsonData.errors[0].message === 'Invalid token provided') {
throw new InvalidRegistryTokenError();
}

if (isDebug) {
this.logFailure(jsonData.errors);
}
throw new APIError(
jsonData.errors.map(e => e.message).join('\n'),
this.cleanRequestId(response?.headers?.get('x-request-id')),
);
}

Expand All @@ -243,32 +280,6 @@ export default abstract class BaseCommand<T extends typeof Command> extends Comm
};
}

handleFetchError(error: unknown): never {
if (typeof error === 'string') {
return this.error(error);
}

if (error instanceof Error) {
if (isClientError(error)) {
const errors = error.response?.errors;

if (Array.isArray(errors) && errors.length > 0) {
return this.error(errors[0].message, {
ref: this.cleanRequestId(error.response?.headers?.get('x-request-id')),
});
}

return this.error(error.message, {
ref: this.cleanRequestId(error.response?.headers?.get('x-request-id')),
});
}

return this.error(error);
}

return this.error(JSON.stringify(error));
}

async require<
TFlags extends {
require: string[];
Expand All @@ -281,20 +292,25 @@ export default abstract class BaseCommand<T extends typeof Command> extends Comm
);
}
}
}

class ClientError extends Error {
constructor(
message: string,
public response: {
errors?: readonly GraphQLError[];
headers: Headers;
},
) {
super(message);
}
}
readJSON(file: string): string {
// If we can't parse it, we can try to load it from FS
const exists = existsSync(file);

function isClientError(error: Error): error is ClientError {
return error instanceof ClientError;
if (!exists) {
throw new FileMissingError(
file,
'Please specify a path to an existing file, or a string with valid JSON',
);
}

try {
const fileContent = readFileSync(file, 'utf-8');
JSON.parse(fileContent);

return fileContent;
} catch (e) {
throw new InvalidFileContentsError(file, 'JSON');
}
}
}
Loading
Loading