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

Restrictive type on callbacks that are guaranteed to error #1613

Closed
Nemo157 opened this issue Jan 7, 2015 · 7 comments
Closed

Restrictive type on callbacks that are guaranteed to error #1613

Nemo157 opened this issue Jan 7, 2015 · 7 comments
Labels
Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@Nemo157
Copy link

Nemo157 commented Jan 7, 2015

Motivation

It's sometimes very useful to be able to define callbacks that guarantee an error. For example; transforming errors in when.js promises to hide implementation details:

function stuff(): when.Promise<Item> {
    return when.reject<Item>(new Error('Oh noes!'));
}

export function tryStuff(): when.Promise<Item> {
    return stuff().catch(err => {
        log.error('Internal error', err);
        throw new InternalError();
    });
}

Unfortunately the compiler derives the type of the error handler to be (err: any) => void and propagates the return type through the definition of catch resulting in

error TS2012: Cannot convert 'when.Promise<void>' to 'when.Promise<Item>'

Even if the definition of tryStuff is changed to force the result to the correct type

return stuff().catch<Item>(err => {
    log.error('Internal error', err);
    throw new InternalError();
});

the same type is derived for the error handler and you instead get

error TS2082: Supplied parameters do not match any signature of call target:
        Call signatures of types '(err: any) => void' and '(reason: any) => when.Promise<Item>' are incompatible.

Details

A fully specified example of the same behaviour is

function run<T>(callback: () => T): T {
    return callback();
}

run<boolean>(() => {
    throw new Error('Failure');
});

which gives the error

error TS2082: Supplied parameters do not match any signature of call target:
        Call signatures of types '() => void' and '() => boolean' are incompatible.

Two possible solutions are returning null to trick the compiler into giving the callback a lenient type (I guess it defaults to any when the only return returns a null), or explicitly telling the compiler that the callback can return anything.

run<boolean>(() => {
    throw new Error('Failure');
    return null;
});

run<boolean>(<() => any>(() => {
    throw new Error('Failure');
}));

The first runs afoul of unreachable statement lints. The second is just really horrible.

Seeing as the lints are capable of determining that there is no way to return from this callback, it would be nice if the compiler could also check this and give the callback the type () => any when it can guarantee there is no possibility of returning.

@danquirk
Copy link
Member

danquirk commented Jan 7, 2015

Why do you need to explicitly specify the type argument? Your run example works without error if you just rely on type inference:

run(() => { // no <boolean>, no error
    throw new Error('Failure');
});

@Nemo157
Copy link
Author

Nemo157 commented Jan 7, 2015

Why do you need to explicitly specify the type argument?

It's just a small example to show the behaviour without any external dependencies, I'm struggling to think of a smaller realistic example than the promises one. An alternative that has (slightly) more semantic meaning and still errors is

var result: boolean;
result = run(() => {
    throw new Error('Failure');
});

A longer fully specified example with some really badly implemented promises is

class Promise<T> {
    constructor(private value: T, private err: any) {}
    catch<U>(callback: (err: any) => U): Promise<U> {
        try {
            return new Promise<U>(callback(this.err), null);
        } catch (err) {
            return new Promise<U>(null, err);
        }
    }
}

function reject<T>(err: any): Promise<T> { return new Promise<T>(null, err); }

class Item { name: string }

function stuff(): Promise<Item> {
    // Call into the database, whatever,
    // get back an exception that contains internal implementation details.
    return reject<Item>(new Error('Oh noes!'));
}

function tryStuff(): Promise<Item> {
    // Hide the internal implementation details by transforming the error.
    return stuff()
        .catch(err => { throw new Error('Something went wrong'); });
}

@Nemo157
Copy link
Author

Nemo157 commented Jan 7, 2015

Interestingly this works

var result: boolean;
result = run((): boolean => {
    throw new Error('Failure');
});

but this errors

var result: boolean;
result = run((): boolean => {
    console.log('running');
    throw new Error('Failure');
});

with

error TS2355: A function whose declared type is neither 'void' nor 'any' must return a value or consist of a single 'throw' statement.

so there is some (very limited) support for this with explicit typing. The error does give me the most succinct syntax currently available to do this

var result: boolean;
result = run((): any => {
    console.log('running');
    throw new Error('Failure');
});

but it would still be nice if the any return type could be inferred.

@danquirk
Copy link
Member

danquirk commented Jan 7, 2015

Yeah there's an exception specifically for that single statement pattern (ex a function body of new Error("Not Yet Implemented")) but nothing more complicated that affects control flow analysis or type inference.

@RyanCavanaugh
Copy link
Member

I think we're considering some basic flow analysis for e.g. use before declaration, at which point we can probably extend that rule to "functions which throw on all return paths".

@danquirk danquirk added the Suggestion An idea for TypeScript label Jan 20, 2015
@Artazor
Copy link
Contributor

Artazor commented Apr 14, 2015

@RyanCavanaugh Exactly!
Eagerly awaiting it.

For now I'm using the following workaround with promises:

var somePromise: Promise<T> = request<T>(args).catch((error: any): Promise<T> => {
    console.log(error);
    return Promise.reject<T>(reason);
});

instead of (desirable)

var somePromise: Promise<T> = request<T>(args).catch((error: any): T => {
    console.log(error);
    throw reason;
});

@mhegazy
Copy link
Contributor

mhegazy commented Dec 9, 2015

The code references above should not be an error in TypeScript 1.8.

var result: boolean;
result = run((): number => {
    console.log('running');
    throw new Error('Failure');
});

@mhegazy mhegazy closed this as completed Dec 9, 2015
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Dec 9, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants