12 ms·

I don't think this is right. One code smell here is that your `chapterPromises` is no longer a set of promises for chapters. You've had to turn exceptions into

I don't think this is right. One code smell here is that your `chapterPromises` is no longer a set of promises for chapters. You've had to turn exceptions into objects to work around the unhandled promise behaviour. It also looks like it'll display chapter 3 if chapter 2 fails to load, which seems wrong. At that point you've failed to display the sequence of chapters.

horsawlarway 4y ago
> I don't think this is right. One code smell here is that your `chapterPromises` is no longer a set of promises for chapters. You've had to turn exceptions into objects to work around the unhandled promise behaviour. I mean, or you do any sort of sensible thing, like automatically retry the failed request (with backoff and a cap) directly in the catch, where the error is local, understood, and you have the information to handle it. But in either case - you're right that it's no longer `Promise<chapter>`, it's now `Promise<Option<chapter, err>>` But... that's not a problem. And it's a much better way to indicate to other sections of the code what has happened than changing code paths completely with an exception or unhandled rejection. Using something like an `Option` (or `Either`, or `Result` - depending on what library/languages you're familiar with) is a pretty sound way to handle errors in a pipeline, and is routinely found across basically every functional language out there.
spankalee 4y ago
It depends so much on the app and behavior you want - to show partial data or not, to do things in sequence. What do chapters stand in for in this example? Yeah, in this case I thought it might be nice to show a "failed to load" message for each failure. Maybe this is a chronological Mastodon timeline, or a photo gallery. Do you want to error the whole UI for one failed object? Maybe But trying to do something useful with all failures it's a general rule that I think guides towards good thinking about UX and fewer unhandled rejection warnings.

horsawlarway 4y ago

> I don't think this is right. One code smell here is that your `chapterPromises` is no longer a set of promises for chapters. You've had to turn exceptions into objects to work around the unhandled promise behaviour. I mean, or you do any sort of sensible thing, like automatically retry the failed request (with backoff and a cap) directly in the catch, where the error is local, understood, and you have the information to handle it. But in either case - you're right that it's no longer `Promise<chapter>`, it's now `Promise<Option<chapter, err>>` But... that's not a problem. And it's a much better way to indicate to other sections of the code what has happened than changing code paths completely with an exception or unhandled rejection. Using something like an `Option` (or `Either`, or `Result` - depending on what library/languages you're familiar with) is a pretty sound way to handle errors in a pipeline, and is routinely found across basically every functional language out there.

spankalee 4y ago

It depends so much on the app and behavior you want - to show partial data or not, to do things in sequence. What do chapters stand in for in this example? Yeah, in this case I thought it might be nice to show a "failed to load" message for each failure. Maybe this is a chronological Mastodon timeline, or a photo gallery. Do you want to error the whole UI for one failed object? Maybe But trying to do something useful with all failures it's a general rule that I think guides towards good thinking about UX and fewer unhandled rejection warnings.