Why doesn't awaiting a Promise<never> change reachability?

[nikeee]

TypeScript Version: 3.7.2

Search Terms:

• "Promise"
• await
• reachability analysis
• control flow analysis
• definite assignment analysis

Code

(async () => {
    let b: string;
    let a1 = await Promise.reject(); // returns Promise<never>
    b = ""; // Not unreachable?
    b.toUpperCase(); // Not unreachable?
})();

Expected behavior: As a1 is inferred to be never (e.g. behaves like in the non-promised version), I expected the rest of the code to be marked as unreachable aswell:

function returnNever(): never { throw new Error(); }

(async () => {
    let b: string;
    let a0 = returnNever(); // a0 is never
    b = ""; // Unreachable
    b.toUpperCase(); // Unreachable
})();

Actual behavior: The code after the never-returning promise is marked as reachable.

Related Question on StackOverflow: Has more code: https://stackoverflow.com/questions/58732814 Related Issue: https://github.com/microsoft/TypeScript/issues/10973 (although marked as "Working as intended", it was changed later. 3.7.2 behaves like the issue opener expected).

If this is not a bug, what is the background for this behavior?

[andrewbranch]

I think this makes sense, though two things are unclear to me:

• how complex/expensive this would be to implement
• if this comes up in real-world scenarios

Have you found real cases where you have a Promise<never>? Would love to see some motivating examples.

[nikeee]

Consider a logErrorAsync function that returns Promise<never>. It awaits something and then throws an error on every path (or goes into an infinite loop etc.). If the function does not return a Promise<never> but a never instead, it behaves as expected.

As of TS 3.7.3:

declare function fetchB(): Promise<string>;

async function logErrorAsync(err: any): Promise<never> {
    await fetch("/foo/...");
    console.log("Error has been submitted to analytics tool");
    throw new Error(err);
}

function logError(err: any): never {
    console.error(err);
    throw new Error(err);
}

(async () => {
    let b: string;
    try {
        b = await fetchB();
    } catch (err) {
        await logErrorAsync(err); // awaiting Promise<never>
    }
    b.toUpperCase(); // "Error: "b" is used before assignment;" would require workarounds
})();

//...
(async () => {
    let b: string;
    try {
        b = await fetchB();
    } catch (err) {
        logError(err); // returns never
    }
    b.toUpperCase(); // No error, as expected
})();

[andrewbranch]

Yeah, since we actually error on unreachable code, this does seem worthy of the bug label.

[nicolas377]

We're closing in on 2 years of this issue, is this something that the TS team wants to do themselves, or could I take this on as a first PR here?

[andrewbranch]

Backlog = no immediate plans for us to work on it, but

Lack of "Help Wanted" = not really sure if there are major roadblocks to a solution. I think this would be a difficult first PR, but you are welcome to try.

[RuiNtD]

Have you found real cases where you have a Promise<never>? Would love to see some motivating examples.

Also this:

https://github.com/FayneAldan/ArtemisWaypointMigrator/blob/a3f38352518ea5de1d9534dabb951bd8e7dda52c/migrate.ts#L15-L23

async function enterToExit(): Promise<never> {
  console.log("Press [Enter] to exit");
  for await (const event of keypress()) {
    if (event.key == "return") {
      break;
    }
  }
  Deno.exit();
}

[bradenhs]

Here's a real use case for this I ran into today: you want to flush your logs then exit the process e.g.

async function exit(exitCode: number): Promise<never> {
  await Logger.flush();
  process.exit(exitCode);
}

[zefir-git]

I believe the code is unreachable because Promise.reject() throws an error, not because the type is never. How would TypeScript know that the code is unreachable when it does not know that this method can throw an error?

In Java there is the throws keyword which defines what errors a method can throw, e.g

public void errors() throws Exception {
    throw new Exception("Exception!");
}

public void test() {
    this.errors(); // will error because `errors()` can throw `Exception` and you don't handle the error. Your options are to add `throws Exception` to the signature of `test` or handle the exception with try catch
    // unreachable code because possible error not handled
}

I believe this (much-needed IMO) feature has been requested in #13219 and until it's implemented we're stuck with unknown or any in catches and not having any checks for unhandled errors.

[rintaun]

@williamd5 this works properly without promises:

function errors(): never {
  throw new Error("Error!")
}

function test() {
  errors()
  console.log('this is unreachable, and typescript knows it')
}

the throws syntax is a separate, unrelated issue about type inference in a catch clause, whereas this issue is a bug regarding reachability calculation by returning never for known, uncaught errors.

---

I have also run into this problem in the wild, specifically when using zod and a z.never() schema in an async workflow. the specific use case was implementing an ORM that handles PostgreSQL views in addition to standard tables.

the general approach i was taking looked like this...

class Table<TInsert extends z.ZodSchema> {
  // ...
  async insert(data: TInsert): Promise<TInsert extends z.ZodNever : never : TInsert> {
    db.insert(insertSchema.parse(data))
  }
}

[jespertheend]

Have you found real cases where you have a Promise<never>? Would love to see some motivating examples.

I have a TypedMessenger which allows you to send messages to things like Workers or WebSockets. It works a bit like:

const messenger = new TypedMessenger()
const response = await messenger.send.foo()

and response will have the type of whatever the WebSocket is expected to respond with.

However, if it is known in advance that the WebSocket will never respond when foo() is called, then its type will be Promise<never>. It would be useful if you could see that awaiting this call would result in unreachable code.

[darrylnoakes]

Does that leave you with never-to-be-fulfilled promises lying around? Or is it internally constructed so that there are no references to them and they can be garbage collected?

[jespertheend]

I'm implementing the garbage collection using WeakRefs as we speak :)

[jespertheend]

Actually, I just realised what I want to do isn't possible :/ So errors for unreachable code would actually be very convenient here.

[toonvanvr]

I'm also bumping upon this issue, but was able to work around it using return, which is not a clean solution. My language server was giving a 'could be undefined' type error in the calling scope after wrapping repetitive error reporting in an async call.

Original code

async function parseFile(): Promise<File> {
try {
return await something()
} catch (cause) {
const errMsg = 'went wrong'
await dbLog(errMsg, { isError: true })
console.error(errMsg, { cause })
throw new Error(errMsg, { cause })
// unreachable code
}
}

Async call with bug

async function parseFile(): Promise<File | undefined> {
try {
return await something()
} catch (cause) {
await loggedCriticalError('went wrong', { cause })
// incorrectly assumed reachable code
}
}

Async call with bug workaround

async function parseFile(): Promise<File> {
try {
return await something()
} catch (cause) {
// returning an awaited Promise<never> will not add void to the return type of parseFile()
return await loggedCriticalError('went wrong', { cause })
// unreachable code
}
}

The need for logging stuff immediately exists because enterprise code is touched by many hands and the error handling in the parent scope might accidentally toss the error message, while fixing that behaviour could introduce its own set of bugs in legacy code that depends on that behaviour. We all know there's no budget for fixing everything. 💸

The inferred return type changed from File to File | undefined and caused a type error right below it, but in other code it could have propagated into the scope of a colleague and they'd be adding a bunch of useless if/elses, adding complexity where it's not needed.