asyncapi / cli

CLI to work with your AsyncAPI files. You can validate them and in the future use a generator and even bootstrap a new file. Contributions are welcomed!
https://www.asyncapi.com/tools/cli
Apache License 2.0
176 stars 139 forks source link

Having invalid examples in spec fails CLI build #994

Closed Souvikns closed 6 months ago

Souvikns commented 7 months ago

Describe the bug

A clear and concise description of what the bug is.

The script that CLI runs in its build process fetches examples from the spec repository, If any example in the spec is invalid the examples.json file that is being created by the script creates a null value in the script and thus the build fails. Even older versions of the CLI start having issues due to this.

How to Reproduce

The only way to reproduce is if we have an invalid example in the spec repository or by copy/pasting this example.json file.

[
    {
        "name": "Account Service",
        "value": "simple-asyncapi.yml"
    },
    {
        "name": "Account Service",
        "value": "simple.yml"
    },
    {
        "name": "AnyOf example",
        "value": "anyof-asyncapi.yml"
    },
    {
        "name": "AnyOf example",
        "value": "anyof.yml"
    },
    {
        "name": "Application Headers example - (protocols: mqtt)",
        "value": "application-headers-asyncapi.yml"
    },
    {
        "name": "Application Headers example - (protocols: mqtt)",
        "value": "application-headers.yml"
    },
    {
        "name": "Correlation ID Example - (protocols: mqtt)",
        "value": "correlation-id-asyncapi.yml"
    },
    {
        "name": "Correlation ID Example - (protocols: mqtt)",
        "value": "correlation-id.yml"
    },
    {
        "name": "Gemini Market Data Websocket API - (protocols: wss)",
        "value": "websocket-gemini-asyncapi.yml"
    },
    {
        "name": "Gemini Market Data Websocket API - (protocols: wss)",
        "value": "websocket-gemini.yml"
    },
    {
        "name": "Gitter Streaming API - (protocols: https)",
        "value": "gitter-streaming-asyncapi.yml"
    },
    {
        "name": "Gitter Streaming API - (protocols: https)",
        "value": "gitter-streaming.yml"
    },
    {
        "name": "Kraken Websockets API",
        "value": "kraken-websocket-request-reply-message-filter-in-reply-asyncapi.yml"
    },
    {
        "name": "Kraken Websockets API",
        "value": "kraken-websocket-request-reply-multiple-channels-asyncapi.yml"
    },
    {
        "name": "Mercure Hub Example - (protocols: mercure)",
        "value": "mercure-asyncapi.yml"
    },
    {
        "name": "Mercure Hub Example - (protocols: mercure)",
        "value": "mercure.yml"
    },
    {
        "name": "Not example",
        "value": "not-asyncapi.yml"
    },
    {
        "name": "Not example",
        "value": "not.yml"
    },
    {
        "name": "Notifications",
        "value": "operation-security-asyncapi.yml"
    },
    {
        "name": "Notifications",
        "value": "operation-security.yml"
    },
    {
        "name": "OneOf example",
        "value": "oneof-asyncapi.yml"
    },
    {
        "name": "OneOf example",
        "value": "oneof.yml"
    },
    {
        "name": "RPC Client Example - (protocols: amqp)",
        "value": "rpc-client-asyncapi.yml"
    },
    {
        "name": "RPC Client Example - (protocols: amqp)",
        "value": "rpc-client.yml"
    },
    {
        "name": "RPC Server Example - (protocols: amqp)",
        "value": "rpc-server-asyncapi.yml"
    },
    {
        "name": "RPC Server Example - (protocols: amqp)",
        "value": "rpc-server.yml"
    },
    {
        "name": "Slack Real Time Messaging API - (protocols: https)",
        "value": "slack-rtm-asyncapi.yml"
    },
    {
        "name": "Slack Real Time Messaging API - (protocols: https)",
        "value": "slack-rtm.yml"
    },
    {
        "name": "Streetlights App - (protocols: mqtt)",
        "value": "tutorial.yml"
    },
    {
        "name": "Streetlights Kafka API - (protocols: kafka-secure,kafka-secure)",
        "value": "streetlights-kafka-asyncapi.yml"
    },
    {
        "name": "Streetlights Kafka API - (protocols: kafka-secure,kafka-secure)",
        "value": "streetlights-kafka.yml"
    },
    {
        "name": "Streetlights Kafka API - (protocols: kafka-secure,kafka-secure)",
        "value": "streetlights-operation-security-asyncapi.yml"
    },
    {
        "name": "Streetlights Kafka API - (protocols: kafka-secure,kafka-secure)",
        "value": "streetlights-operation-security.yml"
    },
    {
        "name": "Streetlights MQTT API - (protocols: mqtt)",
        "value": "streetlights-mqtt-asyncapi.yml"
    },
    {
        "name": "Streetlights MQTT API - (protocols: mqtt)",
        "value": "streetlights-mqtt.yml"
    },
    null
]

Expected behavior

If we have any invalid example we should not generate the example.json with a null value.

smoya commented 7 months ago

Sharing this Slack thread that adds more context and shows the consequences of this issue https://asyncapi.slack.com/archives/CQVJXFNQL/p1702376239408029?thread_ts=1702289410.144129&cid=CQVJXFNQL

smoya commented 7 months ago

The issue is related with this line https://github.com/asyncapi/cli/blob/7259f7c71060b86529028463c3998568adb15457/scripts/fetch-asyncapi-example.js#L73C9-L73C9

In the case the parser cant parse a document, document property will be null and the early return will happen, adding a null element to the array of examples.

The point here is that a decision must be taken about erroring (quitting the app with an error message) when that happens or rather skip the example. The second option is what the code is supposed to be doing, however we would need to avoid adding that null value to the final array somehow (this is part of the task to solve this).

Considering that all examples should be valid if https://github.com/asyncapi/spec/issues/957 moves forward, I would say we go with the second option.

Shiva953 commented 7 months ago

The issue is related with this line https://github.com/asyncapi/cli/blob/7259f7c71060b86529028463c3998568adb15457/scripts/fetch-asyncapi-example.js#L73C9-L73C9

In the case the parser cant parse a document, document property will be null and the early return will happen, adding a null element to the array of examples.

The point here is that a decision must be taken about erroring (quitting the app with an error message) when that happens or rather skip the example. The second option is what the code is supposed to be doing, however we would need to avoid adding that null value to the final array somehow (this is part of the task to solve this).

Considering that all examples should be valid if asyncapi/spec#957 moves forward, I would say we go with the second option.

I would like to implement this

smoya commented 7 months ago

The issue is related with this line https://github.com/asyncapi/cli/blob/7259f7c71060b86529028463c3998568adb15457/scripts/fetch-asyncapi-example.js#L73C9-L73C9 In the case the parser cant parse a document, document property will be null and the early return will happen, adding a null element to the array of examples. The point here is that a decision must be taken about erroring (quitting the app with an error message) when that happens or rather skip the example. The second option is what the code is supposed to be doing, however we would need to avoid adding that null value to the final array somehow (this is part of the task to solve this). Considering that all examples should be valid if asyncapi/spec#957 moves forward, I would say we go with the second option.

I would like to implement this

For the record, the following conversation is happening: https://asyncapi.slack.com/archives/CQVJXFNQL/p1702544253816909?thread_ts=1702289410.144129&cid=CQVJXFNQL

asyncapi-bot commented 6 months ago

:tada: This issue has been resolved in version 1.2.31 :tada:

The release is available on:

Your semantic-release bot :package::rocket: