Fix theme serve failing when the port is already being used

[karreiro]

Fix the theme serve service by rescuing the Errno::EADDRINUSE and showing a more friendly message for users (issue: #1620).

WHY are these changes introduced?

Before this change, Errno::EADDRINUSE was not being handled and the standard error was appearing for users.

WHAT is this pull request doing?

Now, this PR handles the Errno::EADDRINUSE error, and shows a more friendly message for users.

How to test your changes?

1. Run shopify theme serve service
2. Open another Terminal window
3. Run the shopify theme serve again

Here's the previous behavior:

Here's how it actual behavior (with this PR applied):

Post-release steps

None.

Update checklist

• I've added a CHANGELOG entry for this PR (if the change is public-facing)
• I've considered possible cross-platform impacts (Mac, Linux, Windows).
• I've left the version number as is (we'll handle incrementing this when releasing).
• I've included any post-release steps in the section above.

[karreiro]

Hey @macournoyer! I've opened the PR as a draft because I'd like to check your thoughts about this approach before proceeding with the unit tests :)

On this PR, I'm following the #1620 call and suggesting the use of a different port.

However, I believe that working on multiple themes, at the same time, can be something frequent for users that create templates.

Thus, I wondered if, by convention, we could increment the port (e.g., 9293, 9294, and so on) when 9292 is already in use. I believe it would make users' life a bit easier, but I'm not sure if it's a too invasive convention.

What do you think?

[macournoyer]

Wonderful! Thanks for refactoring the messages too 😄

Thus, I wondered if, by convention, we could increment the port (e.g., 9293, 9294, and so on) when 9292 is already in use. I believe it would make users' life a bit easier, but I'm not sure if it's a too invasive convention.

I'm also on the edge if it's too invasive. Seems risky to just increment the port.
However, a good approach to automatically do this could be to let the OS pick an ephemeral port, this can be done by setting port to 0: https://stackoverflow.com/a/201528.

But that will change the current behavior of the command. So might be better to stick w/ your current approach for this PR.

[pepicrft]

Besides the comments, the code looks great. Thanks for making this contribution to the project @karreiro 👏🏼 🚀

[pepicrft]

I'm looping in @MeredithCastile to get her review on this one.

[MeredithCastile]

I want to use this issue as an opportunity to talk about error messaging patterns in the Shopify CLI. @kwringe has identified 4 patterns that show today. Obviously, it'd be best to consolidate these into one consistent pattern.

Today the 4th pattern listed in Kate's table is just is for commands that aren't recognized. One idea is that we could expand that pattern for all error messages. Red box for error, green box for "try this," purple box for "did you mean." Perhaps a neon blue for any suggested commands (ex: HEX 00F0FF). With that pattern, the error message for this issue could be rendered as:

In a "no color" mode or for people with red/green colorblindness, it'd still work:

This pattern follows best practices (and our CLI guidelines in Polaris) by outputting only 1 idea per line. It's definitely visually heavy, but can be easily parsed and isn't easily buried when surrounded by, say, printed logs.

It's certainly more scannable than a table-based alternative:

Open to any and all feedback here! cc: @PhilMcClelland, @megthesmith, @stephszeto, @hannachen

[gonzaloriestra]

@MeredithCastile great points! Just wanted to point out that not everyone uses a dark mode. So in my case, the red font is easy to read:

Anyway, I think the patterns you suggest would work for both modes.

[kwringe]

Red font is great in that it attracts attention, but having too much text is red font can be difficult to read.

I like the idea of the try this box, and would like to see how devs respond to it.

[karreiro]

+1 from my side for the box-based-approach as well! 🚀

[karreiro]

Hey @macournoyer @pepibumur, this PR is ready for review (the images in the description are updated). I've added tests, fixes for the comments above, and the frames for the error message, following @MeredithCastile feedback :)

[macournoyer]

Awesome! Just a minor issue w/ casing, and a suggestion. 🚀