opticdev / optic

OpenAPI linting, diffing and testing. Optic helps prevent breaking changes, publish accurate documentation and improve the design of your APIs.
https://useoptic.com
MIT License
1.35k stars 79 forks source link

`optic run` fails to report exit code when the diff fails #2797

Closed balint-backmaker closed 6 months ago

balint-backmaker commented 6 months ago

Describe the bug

When running optic run --severity error <path-to-openapi-file> it does not properly report a negative exit code when the diff fails.

Example script that we run on circleci:

# simulate GH action run
echo "{\"pull_request\": {\"number\": ${PR_NUMBER}}}" > ./pr-event.json
export GITHUB_EVENT_PATH="./pr-event.json"

set -xe

optic run --severity error "${CHANGED_SPECS}"

echo "Last exit code was: $?"

And the output:

+ optic run --severity error /home/circleci/project/docs/openapi/openapi.yaml
Optic matched 1 OpenAPI specification file:
/home/circleci/project/docs/openapi/openapi.yaml

--------------------------------------------------------------------------------------------------

 ┌─────────────────────────────────┐
 │  [1]      Optic Cloud      [2]  │
 └───┬─────────────────────────▲───┘
     │Compare            Update│
 ┌───▼─────────────────────────┴───┐
 │           Local specs           │
 └─────────────────────────────────┘

 [1]: `gitbranch:main` tag
 [2]: `gitbranch:feat/my-feature-branch` tag

--------------------------------------------------------------------------------------------------

| undefined [docs/openapi/openapi.yaml]
| Diff failed:
| Invalid specification: Error: invalid openapi: paths > /api/v1/funnel/{kind}/questions > options must NOT have additional properties
33 |               schema:
34 |                 $ref: '#/components/schemas/HTTPValidationError'
35 |       x-tier: 1
36 |       cache-ttl: 60  # <- this is what is causing failure, rightfully so!
37 | components:
38 |   schemas:
39 |     HTTPValidationError:
/home/balint/git/buyback-funnel/docs/openapi/openapi.yaml

🔧 Customize your governance rules: https://www.useoptic.com/docs/lint-openapi
+ echo 'Last exit code was: 0'
Last exit code was: 0

To Reproduce Use this openapi.yaml file and run optic run --severity error <path-to-file>.

openapi: 3.1.0
x-optic-standard: '@back-market/back-market-primary-standard'
info:
  title: buyback-funnel API
  version: '0.1'
  description: Service hosting the buyback questions funnel.
  contact:
    name: my-squad
    email: my-squad@backmarket.com
    url: https://example.com
    x-slack: '#bot-buyback-release'
externalDocs:
  description: TechDocs
  url: https://example.com
servers: []
paths:
  /api/v1/funnel/{kind}/questions:
    options:
      tags:
        - Public
      summary: Options Questions
      description: OPTIONS must be allowed on this route, in order to respond to pre-flight
        requests performed by API consumers.
      operationId: options_questions
      parameters: [] # removed for brevity
      responses:
        '204':
          description: Successful Response
        '422':
          description: Validation Error
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/HTTPValidationError'
      x-tier: 1
      cache-ttl: 60  # <- this is what is causing failure, rightfully so!
components:
  schemas:
    HTTPValidationError:
      properties:
        detail:
          items:
            $ref: '#/components/schemas/ValidationError'
          type: array
          title: Detail
      type: object
      title: HTTPValidationError
    ValidationError:
      properties:
        loc:
          items:
            anyOf:
              - type: string
              - type: integer
          type: array
          title: Location
        msg:
          type: string
          title: Message
        type:
          type: string
          title: Error Type
      type: object
      required:
        - loc
        - msg
        - type
      title: ValidationError

Expected behavior I expect optic run to also fail the exit code when the underlying diff fails for any reason. Currently if it's an invalid spec, then optic diff fails correctly but optic run does not catch this fact.

This should always be error severity regardless of the --severity set in the flag as it's clearly an invalid spec.

Screenshots N/A

Details (please complete the following information):

Additional context N/A

niclim commented 6 months ago

Hi,

I believe this should be fixed in 0.54.12