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

Improved support for multiple response types/204 no content #10

Open
EventuallyEven opened this issue Jun 8, 2021 · 3 comments
Open

Comments

@EventuallyEven
Copy link

EventuallyEven commented Jun 8, 2021

Given the following definition

"/someurl": {
  "get": {
	"operationId": "someId",
	"tags": ["tag"],
	"parameters": [
	  {
		"name": "thing",
		"in": "path",
		"required": true,
		"schema": {
		  "type": "string"
		}
	  }
	],
	"responses": {
	  "200": {
		"description": "Yes",
		"content": {
		  "application/json": {
			"schema": {
			  "$ref": "someschema"
			}
		  }
		}
	  },
	  "204": {
		"description": "Empty"
	  }
	}
  }
}

we get the following interface method in the api

someId(params: {
        pathParams: { thing: string };
    }): Promise<someschema>;

The ACTUAL values that can be returned are someschema OR a Response object with status 204.
It would be preferable for the api to reflect this, or, even better, if the actual return value was
someschema or void, i.e.

someId(params: {
        pathParams: { thing: string };
    }): Promise<someschema | void>;
@jhannes
Copy link
Owner

jhannes commented Jun 8, 2021

Currently, the following operation will potentially throw an exception:

"/someurl": {
  "get": {
	"operationId": "someId",
	"tags": ["tag"],
	"parameters": [
	  {
		"name": "thing",
		"in": "path",
		"required": true,
		"schema": {
		  "type": "string"
		}
	  }
	],
	"responses": {
	  "200": {
		"description": "Yes",
		"content": {
		  "application/json": {
			"schema": {
			  "$ref": "someschema"
			}
		  }
		}
	  },
	  "400": {
		"description": "An error occurred"
	  }
	}
  }
}```

It may be hard to support the 204 case without making the 400-case worse. What are your thoughts?

@EventuallyEven
Copy link
Author

EventuallyEven commented Jun 9, 2021

Ideally, error codes would not result in a thrown exception, but a rejecteded promise, regardless of what is declared as possible responses.

Alternatively, maybe any method with JSON as a possible result should result in

someId(params: { <params> }): Promise<someschema | Response>;
This is more correct as handleResponse is
if (contentType && contentType.startsWith("application/json")) { return response.json(); } return response;

@jhannes
Copy link
Owner

jhannes commented Aug 3, 2021

Partially implemented this in 7b8c031

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

No branches or pull requests

2 participants