swagger-api / swagger-codegen

swagger-codegen contains a template-driven engine to generate documentation, API clients and server stubs in different languages by parsing your OpenAPI / Swagger definition.
http://swagger.io
Apache License 2.0
17.08k stars 6.03k forks source link

[typescript/angular2] error handling no checks out error codes... #4373

Open jeusdi opened 7 years ago

jeusdi commented 7 years ago
Description

I've realized typescript/angular2 codegen, generates some strange code related with error handling:

/**
     * Add user
     * 
     * @param user username
     * @param passwd passwd
     */
    public create(user: string, passwd: string, extraHttpRequestParams?: any): Observable<{}> {
        return this.createWithHttpInfo(user, passwd, extraHttpRequestParams)
            .map((response: Response) => {
                if (response.status === 204) {    <<<<<<<<<<<<<<<<<
                    return undefined;
                } else {
                    return response.json();
                }
            });
    }

    /**
     * User exists
     * 
     * @param user username
     */
    public exists(user: string, extraHttpRequestParams?: any): Observable<boolean> {
        return this.existsWithHttpInfo(user, extraHttpRequestParams)
            .map((response: Response) => {
                if (response.status === 204) {    <<<<<<<<<<<<<<<<<<<<<
                    return undefined;
                } else {
                    return response.json();
                }
            });
    }

Why is codegen checking only whether response.status === 204? What about other error codes?

My swagger definition is like:

    @PUT
    @ApiOperation(value = "Add user")
    @ApiResponses(value = {
      @ApiResponse(code = 400, message = "Specified 'username' already exists on Living Commty."),
      @ApiResponse(code = 401, message = "Specified 'username' has no a valid mail form."),
      @ApiResponse(code = 402, message = "Specified 'passwd' is too short.")
    })
    public abstract Response create(...

    @GET
    @ApiOperation(value = "User exists", response = Boolean.class)
    @ApiResponses(value = {
      @ApiResponse(code = 400, message = "Invalid user information specified") 
    })
    public abstract Response exists(
Swagger-codegen version

2.2.3-SNAPSHOT

Swagger declaration file content or url
swagger: '2.0'
info:
  description: desc
  version: 1.0.2
  title: Living API
  termsOfService: 'http://swagger.io/terms/'
  contact:
    name: apiteam@swagger.io
  license:
    name: Apache 2.0
    url: 'http://www.apache.org/licenses/LICENSE-2.0.html'
host: localhost
basePath: /commty/cmng
tags:
  - name: users
schemes:
  - http
paths:
  /users:
    get:
      tags:
        - users
      summary: User exists
      description: ''
      operationId: exists
      produces:
        - application/json
      parameters:
        - name: user
          in: header
          description: username
          required: true
          type: string
      responses:
        '200':
          description: successful operation
          schema:
            type: boolean
        '400':
          description: Invalid user information specified
    put:
      tags:
        - users
      summary: Add user
      description: ''
      operationId: create
      produces:
        - application/json
      parameters:
        - name: user
          in: header
          description: username
          required: true
          type: string
        - name: passwd
          in: header
          description: passwd
          required: true
          type: string
      responses:
        '400':
          description: Invalid user information specified
Command line used for generation

java -jar ".\swagger-codegen-cli\target\swagger-codegen-cli.jar" generate -i http://localhost:8082/commty/cmng/swagger.json -l typescript-angular2 -v

wing328 commented 7 years ago

@jeusdi HTTP status code 204 means "no content" (ref: https://httpstatuses.com/204), which explains why undefined is returned.

jeusdi commented 7 years ago

@wing328 My question was around: Why does codegen only takes care of one type of error response instead of taking @ApiResponses annotation content:

@ApiResponses(value = { @ApiResponse(code = 400, message = "Specified 'username' already exists on Living Commty."), @ApiResponse(code = 401, message = "Specified 'username' has no a valid mail form."), @ApiResponse(code = 402, message = "Specified 'passwd' is too short.") })

Moreover, what about exceptions thrown by create and exists methods:

@Path(value = "/users")
@Produces({"application/json"})
@Api(value = "/users")
public interface IUserCommtyEndpoint {

    @PUT
    @ApiOperation(value = "Add user")
    @ApiResponses(value = {
      @ApiResponse(code = 400, message = "Specified 'username' already exists on Living Commty."),
      @ApiResponse(code = 401, message = "Specified 'username' has no a valid mail form."),
      @ApiResponse(code = 402, message = "Specified 'passwd' is too short.")
    })
    public abstract Response create(
        @HeaderParam("user")
        @ApiParam(value = "username", required = true)
        @NotNull(message = "user.username.notnull")
        @Pattern(regexp="[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?", message="{user.username.email}")
            String username,
        @HeaderParam("passwd")
        @ApiParam(value = "passwd", required = true)
        @NotNull(message = "user.passwd.notnull")
        @Size(min = 6, max = 20, message = "{username.passwd.size}")
            String passwd
    ) throws UsernameAlreadyExistsCommtyException, RepositorySystemException;

    @GET
    @ApiOperation(value = "User exists", response = Boolean.class)
    @ApiResponses(value = {
      @ApiResponse(code = 400, message = "Invalid user information specified") 
    })
    public abstract Response exists(
        @HeaderParam("user")
        @ApiParam(value = "username", required = true)
        @NotNull(message = "user.username.notnull")
        @Pattern(regexp="[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?", message="{user.username.email}")
            String username
    );

}

These exceptions are UsernameAlreadyExistsCommtyException, RepositorySystemException, javax.validation.ValidationException. All of them are handled by JAXRS mappers. For example:

@Provider
public class CommtyExceptionMapper implements ExceptionMapper<UsernameAlreadyExistsCommtyException> {

    @Override
    public Response toResponse(UsernameAlreadyExistsCommtyException exception) {
        return Response
                .status(Response.Status.BAD_REQUEST.getStatusCode())
                .type(MediaType.APPLICATION_JSON)
                .entity(ErrorMessage.create(exception.getCode(), exception.getMessage()))
                .build();
    }

}

As you can see, I'm embedding an ErrorMessage {code, message} on body, and I would like to handle them on typescript\angular2 code...

Any approach to handle that?

bobvanderlinden commented 7 years ago

I ran into the issue that I want to retrieve the status of the response. More specifically I'd like to know when 401 was retrieved. I cannot determine this from the body. This issue seemed to match my problem.

Just to note, this isn't just about errors, as 204 is not an error, but it is about accessing the response. The distinction can be done using the response's status code. To still make this strongly typed I'd like to suggest the following:

Wouldn't it be more correct to generate InlineBody200 in addition to InlineResponse200. InlineBody200 represents the schema and InlineResponse200 is the response itself. The response itself is like a Reponse from angular2, except that its .json() returns InlineBody200.

In addition a type can be generated that is a union of the possible responses:

type InlineResponse = InlineResponse200 | InlineResponse401 | etc

The call in DefaultApi can return a Observable<InlineResponse>. is functions return a InlineResponse200 | InlineResponse401 | etc.

That way you'd have all information that angular2 supplies, but it's still strongly typed.