Bad Sad error when generating openApi definition file

[hexed fox]

I am getting this Bad Sad error. Same works with Ballerina 2201.4.1. For now, i will generating using that Bal version.

reported Issue: https://github.com/ballerina-platform/openapi-tools/issues/1357
Service code : https://github.com/wso2-enterprise/choreo-marketplace/blob/main/marketplaceService.bal

 ~/…/markertplace-openapi/to-share/latest|$ bal openapi -i /Users/hasithah/work/choreo-marketplace/marketplaceService.bal
ballerina: Oh no, something really went wrong. Bad. Sad.

We appreciate it if you can report the code that broke Ballerina in
https://github.com/ballerina-platform/ballerina-lang/issues with the
log you get below and your sample code.

We thank you for helping make us better.

[2023-05-17 13:52:56,809] SEVERE {b7a.log.crash} - null
java.lang.NullPointerException
    at io.ballerina.openapi.converter.service.OpenAPIResponseMapper.lambda$addResponse$9(OpenAPIResponseMapper.java:774)
    at java.base/java.util.LinkedHashMap.forEach(LinkedHashMap.java:684)
    at io.ballerina.openapi.converter.service.OpenAPIResponseMapper.lambda$addResponse$10(OpenAPIResponseMapper.java:768)
    at java.base/java.util.Optional.ifPresent(Optional.java:183)

...

@fickle stratus knows details

[safe harness]

I could narrow down the issue to the following code. There may be more issues.

import ballerina/http;

service / on new http:Listener(9090) {
    resource function get foo() returns string|error|http:InternalServerError {
        return "Hello, World!";
    }
}

For some reason, when you have both http:InternalServerError and error types in the return signature, open api generation fails. @obtuse halo @blissful kelp

@hexed fox My recommendation is this.
-- Do not use the error type in the return signature of services. Use appropriate union of HTTP status code responses.
-- I would use http:InternalServerError instead of error type in the return signature of resources.

[hexed fox]

I agree - i should not be having both together. As you said better use specific union of HTTP code responses. Thanks @safe harness for the investigation. I will try updating the service.

[obtuse halo]

@safe harness since having http:InternalServerError and error altogether is semantically valid (although not recommended), we should gracefully handle the spec generation IMO.

We currently generate 500 responses for both types. So incase we have both together, generating a single 500 response in the contract would be the correct approach I guess.

@granite anvil @blissful kelp thoughts?

[blissful kelp]

Yeah we will need to fix this within tool, and +1 for align with one single 500 response mapping and @granite anvil would like to know whether there is any different details we need to display in spec generation for error and http:InternalServerError apart from that their response body details?

[hexed fox]

@blissful kelp as a test, I did following. I removed all error parts whenever i have a http:InternalServerError in the service. The service now has compilation errors. I ran the command bal openapi -i /Users/hasithah/work/choreo-marketplace/marketplaceService.bal and it gave no error. Interestingly, it did not create any file also!

[blissful kelp]

Code generation doesn't proceed, when the code has compilation issues, But anyhow we need to inform user that the reason for not generating file instead of silently ignored. We will add an error message for code generation. 🙂

[hexed fox]

Yes please