Repository: Redocly/redocly-cli
Pull request #798 (2022-08-08 12:30:05)

fix: incorrect return codes when exit with error
## What/Why/How?
Resolve an issue where we always return a 0 exit code, even when the result is not successful. 
Therefore, we decided to return code 1 in the handleError function to cover move cases then mentioned in the initial issue.

## Reference
Fixed #547

## Testing
Tested locally.

## Screenshots (optional)
N/a

## Check yourself

- [x] Code is linted
- [ ] Tested with redoc/reference-docs/workflows
- [x] All new/updated code is covered with tests

## Security

- [x] Security impact of change has been considered
- [x] Code follows company security practices and guidelines


github-actions[bot]: <!-- jest coverage report action for options with hash 3339ae53a38c44382c0fb4491bdb282d -->

## Coverage report 



| <div title="Status of coverage:&#10; 🟢 - ok&#10; 🟡 - slightly more than threshold&#10; 🔴 - under the threshold">St.<sup>:grey_question:</sup></div> | Category   | Percentage | Covered / Total |
| :----------------------------------------------------------------------------------------------------------------------------------------------------: | :--------- | :--------- | :-------------: |
|                                                                           🟡                                                                           | Statements | 68.45%     |    2864/4184    |
|                                                                           🟡                                                                           | Branches   | 61.83%     |    1584/2562    |
|                                                                           🟡                                                                           | Functions  | 60%        |     459/765     |
|                                                                           🟡                                                                           | Lines      | 68.26%     |    2654/3888    |


<details><summary>Show files with reduced coverage 🔻</summary>
<br/>

| <div title="Status of coverage:&#10; 🟢 - ok&#10; 🟡 - slightly more than threshold&#10; 🔴 - under the threshold">St.<sup>:grey_question:</sup></div> | File                                                        | Statements | Branches | Functions | Lines  |
| :----------------------------------------------------------------------------------------------------------------------------------------------------: | :---------------------------------------------------------- | :--------- | :------- | :-------- | :----- |
|                                                                           🟢                                                                           | <div title="cli/src/commands/lint.ts">`...` / lint.ts</div> | 80%        | 81.25%   | 60%       | 81.48% |
|                                                                           🔴                                                                           | cli/src/utils.ts                                            | 31.85%     | 13.1%    | 26.67%    | 31.85% |

</details>

## Test suite run success
431 tests passing in 75 suites.

<p align="right">Report generated by <a href="https://github.com/ArtiomTr/jest-coverage-report-action">🧪jest coverage report action</a> from e532a5c2dd913e5573140312be6dd0363e5ad6eb</p>


Patch:
diff --git a/packages/cli/src/__mocks__/utils.ts b/packages/cli/src/__mocks__/utils.ts
index 6ef72060..8c8b11f5 100644
--- a/packages/cli/src/__mocks__/utils.ts
+++ b/packages/cli/src/__mocks__/utils.ts
@@ -1,5 +1,5 @@
 export const getFallbackEntryPointsOrExit = jest.fn((entrypoints) =>
-  entrypoints.map(() => ({ path: '' }))
+  entrypoints.map((path: string) => ({ path }))
 );
 export const dumpBundle = jest.fn(() => '');
 export const slash = jest.fn();
diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts
index d0f990c1..3c7cd710 100644
--- a/packages/cli/src/commands/lint.ts
+++ b/packages/cli/src/commands/lint.ts
@@ -107,7 +107,6 @@ export async function handleLint(argv: LintOptions, version: string) {
       const elapsed = getExecutionTime(startedAt);
       process.stderr.write(gray(`${path.replace(process.cwd(), '')}: validated in ${elapsed}\n\n`));
     } catch (e) {
-      totals.errors++;
       handleError(e, path);
     }
   }
diff --git a/packages/cli/src/utils.ts b/packages/cli/src/utils.ts
index 0dc902a6..85702000 100644
--- a/packages/cli/src/utils.ts
+++ b/packages/cli/src/utils.ts
@@ -189,9 +189,10 @@ export function handleError(e: Error, ref: string) {
       );
     } else {
       process.stderr.write(`Something went wrong when processing ${ref}:\n\n  - ${e.message}.\n\n`);
-      throw e;
     }
   }
+  process.exitCode = 1;
+  throw e;
 }
 
 export function printLintTotals(totals: Totals, definitionsCount: number) {


Test patch:
diff --git a/packages/cli/src/__tests__/commands/bundle.test.ts b/packages/cli/src/__tests__/commands/bundle.test.ts
index f54e20fd..ab1ace3a 100644
--- a/packages/cli/src/__tests__/commands/bundle.test.ts
+++ b/packages/cli/src/__tests__/commands/bundle.test.ts
@@ -1,6 +1,7 @@
 import { lint, bundle, getTotals, getMergedConfig } from '@redocly/openapi-core';
 
 import { handleBundle } from '../../commands/bundle';
+import { handleError } from '../../utils';
 import SpyInstance = jest.SpyInstance;
 
 jest.mock('@redocly/openapi-core');
@@ -24,7 +25,7 @@ describe('bundle', () => {
   afterEach(() => {
     (lint as jest.Mock).mockClear();
     (bundle as jest.Mock).mockClear();
-    (getTotals as jest.Mock).mockClear();
+    (getTotals as jest.Mock).mockReset();
   });
 
   it('bundles definitions w/o linting', async () => {
@@ -62,6 +63,12 @@ describe('bundle', () => {
   it('bundles definitions w/ linting', async () => {
     const entrypoints = ['foo.yaml', 'bar.yaml', 'foobar.yaml'];
 
+    (getTotals as jest.Mock).mockReturnValue({
+      errors: 0,
+      warnings: 0,
+      ignored: 0,
+    });
+
     await handleBundle(
       {
         entrypoints,
@@ -116,4 +123,47 @@ describe('bundle', () => {
     exitCb?.();
     expect(processExitMock).toHaveBeenCalledWith(1);
   });
+
+  it('handleError is called when bundles an invalid definition', async () => {
+    const entrypoints = ['invalid.json'];
+
+    (bundle as jest.Mock).mockImplementationOnce(() => {
+      throw new Error('Invalid definition');
+    });
+
+    await handleBundle(
+      {
+        entrypoints,
+        ext: 'json',
+        format: 'codeframe',
+        lint: false,
+      },
+      '1.0.0'
+    );
+
+    expect(handleError).toHaveBeenCalledTimes(1);
+    expect(handleError).toHaveBeenCalledWith(new Error('Invalid definition'), 'invalid.json');
+  });
+
+  it("handleError isn't called when bundles a valid definition", async () => {
+    const entrypoints = ['foo.yaml'];
+
+    (getTotals as jest.Mock).mockReturnValue({
+      errors: 0,
+      warnings: 0,
+      ignored: 0,
+    });
+
+    await handleBundle(
+      {
+        entrypoints,
+        ext: 'yaml',
+        format: 'codeframe',
+        lint: false,
+      },
+      '1.0.0'
+    );
+
+    expect(handleError).toHaveBeenCalledTimes(0);
+  });
 });
diff --git a/packages/cli/src/__tests__/commands/lint.test.ts b/packages/cli/src/__tests__/commands/lint.test.ts
index 8d3d9e15..c741b87d 100644
--- a/packages/cli/src/__tests__/commands/lint.test.ts
+++ b/packages/cli/src/__tests__/commands/lint.test.ts
@@ -120,7 +120,7 @@ describe('handleLint', () => {
     it('should catch error in handleError if something fails', async () => {
       (lint as jest.Mock<any, any>).mockRejectedValueOnce('error');
       await handleLint(argvMock, versionMock);
-      expect(handleError).toHaveBeenCalledWith('error', '');
+      expect(handleError).toHaveBeenCalledWith('error', 'openapi.yaml');
     });
   });