Return type from a route is not being checked against the schema,

[respondsid]

When a Promise is retuened from a @service (in my case its a js service being called from ts controller, js service returns an object which adheres to the return type of the ts controller). When I look at the generated route.ts file aparently no ref is generated for return type and a global handler is generated which simply flushes out the output res.json(data)

Sorting

• I'm submitting a ...

  • [ *] bug report
  • [ ] feature request
  • [ ] support request

• I confirm that I

  • [ ] used the search to make sure that a similar issue hasn't already been submit

Expected Behavior

prior to res.json(data) the generated TS code should enforce the return type contract and reduce the returned object from service (data) to the ts contract

Current Behavior

Currently if a controller returns a Promise of any specific type, and from service if we return a different type (Considering service is a js file), the tsoa module doesnt enforce the outgoing contract on the json object return as part of the route invocation, note that it does perform type based validations in request body.

Possible Solution

In route generator template the promiseHandler should also consider ref: of response type and should try and reduce the service retuened object (data) to the returned type interface.

Steps to Reproduce

Context (Environment)

Version of the library: Version of NodeJS:

• Confirm you were using yarn not npm: [ ]

Detailed Description

Breaking change?

[lukeautry]

This is a good call out and I can see how this would be expected behavior.

The route file already has the information available to validate an object, so I think it's not a huge lift to do validation on the response.

However, validating responses can lead to problems and poor performance. I've worked on APIs where the response contained an array with over 100,000 elements, so running validation on this entire result set will have a non-negligible impact on response time.

I can think of a few paths forward:

• Status quo: don't do response validation
• Progressive enhancement: give tsoa users an option of doing response validation, if it's important to them and if they're willing to deal with the performance impact, whatever the impact may be
• Do response validation for all tsoa users
  • I don't think this is a great option; this could easily result in regressions and breakage in production apps. I'd prefer to not do this.

thoughts @WoH?

[WoH]

What would you do you do if the response does not validate? I am opposed to enabling this behaviour by default, but I could see someone turning this on in development. I'm pretty convinced TS is a better tool to enforce types, but the info is there, may aswell use it.

[respondsid]

So my understanding is that it should throw a type casting error. With the current implementation the type is resolved by grnerated route js. So they type enforcement aldo needs to be therr . I agrre that it csn be configurable however i believe a type based framework should have sn option to enforce typed in/out

[lukeautry]

What would you do you do if the response does not validate?

Yeah, this is a big issue IMO. As an API client, you care that your request is invalid. You don't care that the developer-defined response is invalid; you just want some response to work with.

In cases where you control the server and the client, sure, you may want the request to blow up to preserve the contract. I really think this is something you'd only want in dev, not prod.

[respondsid]

If TSOA has an option to define a response as Promise<T>, I think tsoa should ate least look at T and do an instance of check ? before returning and should blow up with a standard error so that it gets logged and developer gets a notification. If we dont do that and send a completely difference response R our consumer will anyway fail even worst we will only get to know about it when a consumer contacts and tells us (Given that we don't have an automated test covering this).

[WoH]

I really think you should take a look at using (strict) TS to check that before runtime. A response r where r is not of shape T >>>>>>>> no response. The instance check will only work for a very small subset of possible responses.

[vpillinger]

Response validation could be very worthwhile for development purposes. For a trivial example one would want to make sure to not sending "password" field back in a response which it shouldn't be returned in.

Although, it may also make more sense to use a tool like Dredd to run integration tests rather than being done at runtime.

[WoH]

How would that differ from TypeScript checking the return type of the controller method?

[vpillinger]

In my case I want to define public interfaces for my classes in regards to API, but I want to have additional properties in my internal model. For example, I want to ensure that I always drop "password" when I translate my output to the API, but I could foresee-ably do the following:

// API interface that I want for my API docs
export interface UserAPI {
	readonly id: number;
	email: string;
	name: string;
	status?: "Happy" | "Sad";
	phoneNumbers: string[];
}

// My internal model, that contains extra fields.
export class User implements UserAPI {
	id: number;
	password: string;
	email: string;
	name: string;
	status?: "Happy" | "Sad";
	phoneNumbers: string[];
	adminAccessId: number;

	constructor() {
		this.id = 1;
		this.password = "hashedPassword";
		this.email = "[email protected]";
		this.name = "foo barino";
		this.status = "Happy";
		this.phoneNumbers = []
		this.adminAccessId = 1234;
	}

	toAPI(): UserAPI {
		return this; // oops, I just returned the password
	}
}

[WoH]

Thanks for the use case which helps me a lot in understanding the narrowed down issue. Correct me if I'm wrong, but the narrowed down feature request boils down to: The structural typing TS provides around this by default has undesirable properties here because it does not check for excess properties (as strict as we'd like here), a check you would like to have in order to ensure that no excess properties are returned with regards to the return type. I would certainly agree that the default behavior is not the ideal tool here, but, for the reasons mentioned previously, I tend to think runtime checking may not be the best approach either.

Instead, I think a helper may be the best middle ground.

type Exact<T, S> =
  T extends S ? 
  Exclude<keyof T, keyof S> extends never ? 
 T : never : never;


function safe<S, T>(check: Exact<T, S>): S {
	return check;
}

Playground

Wdyt?

[github-actions[bot]]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

[MatthiasKunnen]

@WoH would you be open to adding an option akin to noImplicitAdditionalProperties?

[WoH]

My views on this have (somewhat) shifted. While my concern initially was the performance, I have found approaches that use JSON Schemas to, in fact, improve serialization. I've wanted to reconsider this issue, but I don't have a lot of time to spare for my own experiments or well formulated thoughts here, but if someone wants to look at fastify for example as inspiration, I'm all for that.

E: Since this approach would somewhat violate TypeScript's definition of what conforming to an interface means (w/ regards to excess properties) I agree that it should be feature-flagged.