Marking promises as handled feels like a bad developer experience

[jakearchibald]

I keep getting caught out by this when working with the navigation API.

I'll write some code, and because I'm a 0.5x developer I make mistakes. The result won't behave as I'm expecting, so I'll open the console, and there's no error, leading me to think the problem isn't just due to an error being thrown. However, it is due to an error being thrown, but the navigation API is swallowing the error.

It feels like the API shouldn't behave like this.

[tbondwilkinson]

Which types of mistakes are not being caught?

The one that bothers me most is the return value of navigate() and traverseTo() is not a Promise, but is awaitable. I think we should introduce a then method that throws an Error so that people get Errors when they try and await those return values, and also change the TypeScript type to make then never so that it produces a type error.

[jakearchibald]

Oh, just me doing dumb stuff like:

navigationEvent.intercept({
  handler() {
    // …
    document.body.appned(…);
  }
});

[domenic]

So the problem is we also want to accommodate code like

await navigation.navigate("/foo").committed;

where the developer really doesn't care about the finished promise. Other examples are

navigation.navigate("/foo");
navigation.navigate("/bar"); // this will abort the finished promise for /foo. Do you care?

navigation.navigate("/foo");
handleTransitionResult(navigation.transition.finished);

// One part of your app:
navigation.navigate("/foo");

// Another part of your app:
navigation.addEventListener("navigateerror", (ev) => {
  handleErrorCentrally(ev.error);
});

Maybe what we need is a separate concept of "unhandled navigation rejection", which can take all these things into account, so that if none of them are handled after an event loop turn, that gets logged to the console. That still doesn't solve the double-navigate() case where you don't care about aborting the navigation to /foo, but maybe that's fine? Or we could even special-case "AbortError" DOMExceptions.

This could be done entirely as a DevTools-level thing, I guess, if we don't care about adding some sort of global unhandlednavigateerror handler for programmatic use.

Or we could just say "you really need to handle your finished promises", but I worry that makes a lot of the above code quite ugly. E.g.

const result = navigation.navigate("/foo");
result.finished.catch(() => {});
await result.committed;

is not an invocation you really want to see scattered throughout your codebase.

---

Regarding TypeScript not detecting await navigation.navigate(), I think that's the same problem as TypeScript not detecting await ({}), and should be fixed on either the TypeScript level or via project-specific typing hacks.

I noticed in this TypeScript playground example it gives a sort of warning, with a small dotted underline, saying "await has no effect on this type of expression" and suggesting that I remove the unnecessary await. Maybe there's something you could enable in your config which enforces that more strictly, to fail various builds?

[jakearchibald]

Or we could even special-case "AbortError" DOMExceptions.

I think that's a good idea, and might be useful in other cases too.

I don't have great ideas for the "navigateerror" case. Maybe navigateErrorEvent.preventDefault() could mark the associated event's promises as fully handled?