Nswag: TypeScript: Set prototype of SwaggerException

[nerfpops]

Because the exception that's thrown by the auto generated code doesn't specify its prototype, one can't be certain that a SwaggerException is the error being cought.

Example:

try {
    await (new MyClient()).myMethod()
} catch (err) {
    // Is the err a SwaggerException or something different?
    // We don't know because if the err is a SwaggerException, it's prototype is still Error
}

If SwaggerException got changed into this instead:

export class SwaggerException extends Error {
    message: string;
    status: number; 
    response: string; 
    result: any; 

    constructor(message: string, status: number, response: string, result: any) {
        super();

        this.message = message;
        this.status = status;
        this.response = response;
        this.result = result;

        Object.setPrototypeOf(this, SwaggerException.prototype);
    }
}

It would be possible to write this while also getting intellisense for SwaggerException:

try {
    await (new MyClient()).myMethod()
} catch (err) {
    if(err instanceof SwaggerException)
        console.log("SwaggerException!")
    else
        console.log("something else")
}

Object.setPrototypeOf is supported back to IE11

[berhir]

I noticed this problem today in one of my projects after I updated TypeScript.
The breaking-changes section of the TypeScript wiki has more information about this.
And there are many issues about this too, e.g. https://github.com/Microsoft/TypeScript/issues/12123.

[RicoSuter]

Adding

Object.setPrototypeOf(this, SwaggerException.prototype);

is simple, but I don't like the idea that the code will behave differently in older browsers...

Any idea on how to solve this so that it works in "all" browsers?

[grovesNL]

I don't recommend using instanceof here (or almost anywhere) for type detection like this - it's very brittle and makes assumptions about where this code will run. Anywhere multiple windows/contexts are used (IFrame, etc.) will fail this check, see the rationale behind Array.isArray for example..

If this is necessary, I would add a property to SwaggerException that can be used to test whether it's a SwaggerException or not, and a corresponding static method to the SwaggerException class that tests this property. This is the same idea as Array.isArray.

For example, SwaggerException.isSwaggerException(err) where isSwaggerException checks for the existence of some property and value on the object.

The test could use a property added to every instance like public _type = "SWAGGER_EXCEPTION" (the underscore to let people know not to do this check directly). Then the check becomes public static isSwaggerException(error: any) { return error._type && error._type === "SWAGGER_EXCEPTION"; }

[RicoSuter]

@grovesNL good idea. Thanks

[nerfpops]

Great idea. If isSwaggerException is a User-Defined Type Guard as explained here: https://www.typescriptlang.org/docs/handbook/advanced-types.html we'll get type inference similar to instanceof.

[RicoSuter]

What if we just do not extend Error?

i.e.

export class SwaggerException { ... }

Then err instanceof SwaggerException works... any drawbacks with this?

[grovesNL]

@RSuter I think it's still slightly cleaner to use the isSwaggerException approach. Using instanceof limits some future changes (i.e. we can never change the class name, turn it into a plain object literal, etc.) whereas the duck typing approach just means we have to support isSwaggerException.

[nerfpops]

Wouldn't we still have to import SwaggerException to be able to call SwaggerException.isSwaggerException(err)?

NSwag could perhaps support both instanceof, using RSuters recent suggestion, and for people dealing with iframes, support isSwaggerException?

[grovesNL]

@nerfpops Good point! I edited my comment 😄 NSwag could support instanceof but the limitations still apply to future changes, and I think it would be cleaner just to have one approach.

[RicoSuter]

Ok I think we'll go with this:

• Remove extends Error for instanceof compatibility (for free)
• Add instance property isSwaggerException (for checks when the static method is not available)
• Add static method isSwaggerException which checks if the object has isSwaggerException == true

    class SwaggerException {
        protected isSwaggerException = true; // hidden because it is only used with any

        public static isSwaggerException(obj: any): obj is SwaggerException {
            return obj.isSwaggerException === true;
        }
    }


    var x = {};
    if (SwaggerException.isSwaggerException(x)) {
         ....
    }

    // or

   if (x.isSwaggerException) {
        let y = <SwaggerException>x;
        ....
   }

is this ok for you all?

[nerfpops]

Love it!

[grovesNL]

@RSuter Sounds good

[nerfpops]

But! Without inheriting from Error, won't we loose the stacktrace?

function test() { this.abc = 123 }; try { throw new test() } catch (err) { console.dir(err) }

test doesn't contain a stacktrace but

try { throw Error() } catch (err) { console.dir(err) }

does.

For backward compatibility, SwaggerException would also require a "name"-field. Don't know if this matters. I personally don't really care about the stack trace here, but I think it's good to know that we'll loose this information without inheriting from Error

See this: https://stackoverflow.com/a/4200200/1193236

I guess the safest bet would be grovesNL proposal. Setting the prototype to enable instanceof in the future is going to be a no-brainer when everyone is off IE in 5 - 10 years :)

[RicoSuter]

Good find, thanks. Its more complicated than i thought:

https://stackoverflow.com/questions/33870684/why-doesnt-instanceof-work-on-instances-of-error-subclasses-under-babel-node

[RicoSuter]

So I think for now we just add isSwaggerException method and field and use that instead of instanceof... unless someone has a better solution which also works in older browsers...

[nerfpops]

Nice. You have removed extends Error from the SwaggerException in FileTemplate.cs though?

[RicoSuter]

Sorry - reverted back to extend error...