Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeScript: 200 with file and 204 throws exception #1602

Open
ahmettahasakar opened this issue Sep 13, 2018 · 21 comments
Open

TypeScript: 200 with file and 204 throws exception #1602

ahmettahasakar opened this issue Sep 13, 2018 · 21 comments

Comments

@ahmettahasakar
Copy link

Hello

If i return a 204 nocontent, the typescript throws a swagger exception. However, it is a successfull return code. blobtotext works and then the pipe throws the exception. Am i missing something?

Thank you

@RicoSuter
Copy link
Owner

Can you post some sample Swagger spec + TS output?

@ahmettahasakar
Copy link
Author

"delete": {
        "tags": [
          "HaileeResponses"
        ],
        "operationId": "HaileeResponses_Delete",
        "parameters": [
          {
            "type": "string",
            "name": "id",
            "in": "path",
            "required": true,
            "format": "guid",
            "x-nullable": false
          }
        ],
        "responses": {
          "200": {
            "x-nullable": true,
            "description": "",
            "schema": {
              "type": "file"
            }
          },
          "204": {
            "description": ""
          },
          "400": {
            "description": "Runtime error",
            "schema": {
              "type": "object",
              "additionalProperties": false,
              "properties": {
                "error": {
                  "$ref": "#/definitions/ErrorViewModel"
                }
              }
            }
          }
        }
      }
delete(id: string): Observable<FileResponse | null> {
        let url_ = this.baseUrl + "/HaileeResponses/{id}";
        if (id === undefined || id === null)
            throw new Error("The parameter 'id' must be defined.");
        url_ = url_.replace("{id}", encodeURIComponent("" + id)); 
        url_ = url_.replace(/[?&]$/, "");

        let options_ : any = {
            observe: "response",
            responseType: "blob",
            headers: new HttpHeaders({
                "Content-Type": "application/json", 
                "Accept": "application/json"
            })
        };

        return this.http.request("delete", url_, options_).pipe(_observableMergeMap((response_ : any) => {
            return this.processDelete(response_);
        })).pipe(_observableCatch((response_: any) => {
            if (response_ instanceof HttpResponseBase) {
                try {
                    return this.processDelete(<any>response_);
                } catch (e) {
                    return <Observable<FileResponse | null>><any>_observableThrow(e);
                }
            } else
                return <Observable<FileResponse | null>><any>_observableThrow(response_);
        }));
    }

    protected processDelete(response: HttpResponseBase): Observable<FileResponse | null> {
        const status = response.status;
        const responseBlob = 
            response instanceof HttpResponse ? response.body : 
            (<any>response).error instanceof Blob ? (<any>response).error : undefined;

        let _headers: any = {}; if (response.headers) { for (let key of response.headers.keys()) { _headers[key] = response.headers.get(key); }};
        if (status === 200 || status === 206) {
            const contentDisposition = response.headers ? response.headers.get("content-disposition") : undefined;
            const fileNameMatch = contentDisposition ? /filename="?([^"]*?)"?(;|$)/g.exec(contentDisposition) : undefined;
            const fileName = fileNameMatch && fileNameMatch.length > 1 ? fileNameMatch[1] : undefined;
            return _observableOf({ fileName: fileName, data: <any>responseBlob, status: status, headers: _headers });
        } else if (status === 204) {
            return blobToText(responseBlob).pipe(_observableMergeMap(_responseText => {
            return throwException("A server error occurred.", status, _responseText, _headers);
            }));
        } else if (status === 400) {
            return blobToText(responseBlob).pipe(_observableMergeMap(_responseText => {
            let result400: any = null;
            let resultData400 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver);
            result400 = resultData400 ? Anonymous7.fromJS(resultData400) : new Anonymous7();
            return throwException("A server error occurred.", status, _responseText, _headers, result400);
            }));
        } else if (status !== 200 && status !== 204) {
            return blobToText(responseBlob).pipe(_observableMergeMap(_responseText => {
            return throwException("An unexpected server error occurred.", status, _responseText, _headers);
            }));
        }
        return _observableOf<FileResponse | null>(<any>null);
    }

@RicoSuter
Copy link
Owner

Ah, I think 200 with file + 204 is a special case and may not be correctly handled at the moment...

@RicoSuter RicoSuter changed the title 204 nocontent exception TypeScript: 200 with file and 204 throws exception Sep 13, 2018
@ahmettahasakar
Copy link
Author

There is no file though. I am not sure why it thinks that. I didnt add 200+file either

@RicoSuter
Copy link
Owner

Can you post the C# controller operation signature?

@ahmettahasakar
Copy link
Author

[HttpDelete("{id:guid}")]
        public async Task<IActionResult> Delete(Guid id)
        {
            var entity = _haileeService.GetHaileeResponseOperation(id);
            if (entity == null)
            {
                return NotFoundWithMessage($"Hailee Response Operation not found {id}");
            }
            await _haileeService.DeleteHaileeResponse(id);
            return NoContent();
        }

and i have a IOperationProcessor as below to add the responses as mentioned in #1353

public class SwaggerResponseTypeAppender : IOperationProcessor
    {
        public async Task<bool> ProcessAsync(OperationProcessorContext context)
        {
            var methodResponse = context.MethodInfo.GetCustomAttributes<ProducesResponseTypeAttribute>();
            if (context.OperationDescription.Method != SwaggerOperationMethod.Delete && !methodResponse.Any())
            {
                throw new ApiException(MyExceptionType.MissingResponseType, context.OperationDescription.Operation.OperationId);
            }

            if (context.OperationDescription.Method == SwaggerOperationMethod.Put || context.OperationDescription.Method == SwaggerOperationMethod.Post)
            {
                await AddResponse(context, HttpStatusCode.UnprocessableEntity, typeof(ValidationFailedResultViewModel));
            }

            if (context.OperationDescription.Method == SwaggerOperationMethod.Get || context.OperationDescription.Method == SwaggerOperationMethod.Put)
            {
                await AddResponse(context, HttpStatusCode.NotFound, null);
            }

            if (context.OperationDescription.Method == SwaggerOperationMethod.Delete)
            {
                await AddResponse(context, HttpStatusCode.NoContent, null);
            }

            await AddResponse(context, HttpStatusCode.BadRequest, typeof(MyExceptionResultViewModel));

            return await Task.FromResult(true);
        }

        private async Task AddResponse(OperationProcessorContext context, HttpStatusCode httpStatusCode, Type type)
        {
            if (type != null && !context.SchemaResolver.HasSchema(type, false))
            {
                await context.SchemaGenerator.GenerateAsync(type, null, context.SchemaResolver);
            }
            var response = new SwaggerResponse();
            if (type != null)
            {
                response.Schema = context.SchemaResolver.GetSchema(type, false);
            }
            switch (httpStatusCode)
            {
                case HttpStatusCode.UnprocessableEntity:
                    response.Description = "Validation error";
                    break;
                case HttpStatusCode.BadRequest:
                    response.Description = "Runtime error";
                    break;
                case HttpStatusCode.NotFound:
                    response.Description = "Record not found error";
                    break;
            }
            int httpCode = (int)httpStatusCode;
            var kvp = new KeyValuePair<string, SwaggerResponse>(httpCode.ToString(), response);
            context.OperationDescription.Operation.Responses.Add(kvp);
        }
    }

@RicoSuter
Copy link
Owner

A result of IActionResult automatically adds a 200 file response. You can change that with the [SwaggerResponse] attribute (see docs)

@ahmettahasakar
Copy link
Author

I see, can i remove it with IOperationProcessor?

@RicoSuter
Copy link
Owner

Yes

@ahmettahasakar
Copy link
Author

When I removed 200 from via IOperationProcessor, then the problem is fixed.

New generated code is below, and it doesn't throw exception


    protected processDelete(response: HttpResponseBase): Observable<void> {
        const status = response.status;
        const responseBlob = 
            response instanceof HttpResponse ? response.body : 
            (<any>response).error instanceof Blob ? (<any>response).error : undefined;

        let _headers: any = {}; if (response.headers) { for (let key of response.headers.keys()) { _headers[key] = response.headers.get(key); }};
        if (status === 204) {
            return blobToText(responseBlob).pipe(_observableMergeMap(_responseText => {
            return _observableOf<void>(<any>null);
            }));
        } else if (status === 400) {
            return blobToText(responseBlob).pipe(_observableMergeMap(_responseText => {
            let result400: any = null;
            let resultData400 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver);
            result400 = resultData400 ? Anonymous7.fromJS(resultData400) : new Anonymous7();
            return throwException("A server error occurred.", status, _responseText, _headers, result400);
            }));
        } else if (status !== 200 && status !== 204) {
            return blobToText(responseBlob).pipe(_observableMergeMap(_responseText => {
            return throwException("An unexpected server error occurred.", status, _responseText, _headers);
            }));
        }
        return _observableOf<void>(<any>null);
    }

@fabiante
Copy link

fabiante commented Oct 10, 2018

I encountered the same problem but I can't remove the ProducesResponseType 200 attribute.
How can I solve or workaround this? 🤔

Edit: I am not dealing with a file. This is the part of my swagger.json:

"responses": {
	"200": {
		"description": "Success",
		"schema": {
			"type": "array",
			"items": {
			"$ref": "#/definitions/Incident"
			}
		}
	},
	"204": {
		"description": "Success"
	},
	"401": {
		"description": "Unauthorized: Authentication not valid",
		"schema": {
			"$ref": "#/definitions/Indicents",
			"type": "Indicents",
			"example": {
				// ...
			}
		}
	},
	"404": {
		"description": "Not Found"
	},
	"500": {
		"description": "Internal Server Error: A major internal server problem occurred",
		"schema": {
			"$ref": "#/definitions/Indicents",
			"type": "Indicents",
			"example": {
				// ...
			}
		}
	}
},

fabiante added a commit to fabiante/NSwag that referenced this issue Oct 15, 2018
Enabling this option (false by default) will cause every 2xx status code to be handled as a successful one.
This might be a workaround for issues like RicoSuter#1602
@fabiante
Copy link

fabiante commented Oct 15, 2018

@RSuter I tried creating a workaround for this. In TS it is possible to "just" return everything all 2xx return codes have responded. This is my change: fabiante@7d1ea53

Edit: I figured out my local problem. This fix works and I'll submit it.

@RicoSuter
Copy link
Owner

@fabiante please create a PR so that we can look into it and review it...

@nealsu
Copy link

nealsu commented Aug 16, 2020

I have a similar issue where my .net core API returns:
NoContent() to a Task<IActionResult> controller method response when the search provides no results.
Ok(result) when there are search results.

Which has the following attribute:
[ProducesResponseType(StatusCodes.Status204NoContent)]
[ProducesResponseType(typeof(IEnumerable<SearchResponse>), StatusCodes.Status200OK)]

And produces an exception on receiving a 204, which is very unexpected behaviour. Note the explicit 204 exception block.

if (status === 500) {
	return response.text().then((_responseText) => {
	return throwException("Server Error", status, _responseText, _headers);
	});
} else if (status === 204) {
	return response.text().then((_responseText) => {
	return throwException("Success", status, _responseText, _headers);
	});
} else if (status === 200) {
	return response.text().then((_responseText) => {
	let result200: any = null;
	let resultData200 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver);
	if (Array.isArray(resultData200)) {
		result200 = [] as any;
		for (let item of resultData200)
			result200!.push(SearchResponse.fromJS(item));
	}
	return result200;
	});
} else if (status !== 200 && status !== 204) {
	return response.text().then((_responseText) => {
	return throwException("An unexpected server error occurred.", status, _responseText, _headers);
	});
}

@jeremyVignelles
Copy link
Collaborator

Related to #2995 and #1259

@nealsu
Copy link

nealsu commented Aug 17, 2020

Thanks @jeremyVignelles , do you think it is still related if the controller method has both attributes:

[ProducesResponseType(typeof(IEnumerable<SearchResponse>), StatusCodes.Status200OK)]
and
[ProducesResponseType(StatusCodes.Status204NoContent)]

Is this valid and should the nswag client manage that? I'll have a look at the other tickets and maybe repost there.
The above is supposed to cater for a search that produces no results.

@jeremyVignelles
Copy link
Collaborator

I'd argue that an empty response should return 200 with [], but there are probably APIs out there that work that way (can return both 200 and 204 without content).
I think that for those cases, the 204 should map to a null result. and should not throw.

@Thepriestdude
Copy link

Thepriestdude commented Apr 28, 2021

Do we have any updates on this? I'm having the same problem as @nealsu.
I'm my API I'm specifying [ProducesResponseType(StatusCodes.Status204NoContent)] and returning null in the response but my swaggerProxy throws the following error:

HTTP Response: 



[...].SwaggerException: Success

Status: 204
Response: 

   at [...].xxx(Guid id, CancellationToken cancellationToken) in /.../path.cs:line 2513
   at [...].xxx(Guid id) in /.../path.cs:line 23

@jeremyVignelles
Copy link
Collaborator

no, still up-for-grab

@sabine19
Copy link

Still no solution or workaround for this?

@jeremyVignelles
Copy link
Collaborator

no

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants