Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

Commit

Permalink
Fix theme serve failing when the port is already being used (#1722)
Browse files Browse the repository at this point in the history
Fix `theme serve` failing when the port is already being used (#1722)
  • Loading branch information
karreiro authored Nov 17, 2021
1 parent 3946652 commit 3e7c66d
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 50 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
From version 2.6.0, the sections in this file adhere to the [keep a changelog](https://keepachangelog.com/en/1.0.0/) specification.
## [Unreleased]
### Fixed
* [#1722](https://github.com/Shopify/shopify-cli/pull/1722): Fix `theme serve` failing when the port is already being used

### Fixed
* [#1751](https://github.com/Shopify/shopify-cli/pull/1751): A bug in the app creation flow that caused the CLI to abort when the form validation failed.
Expand Down
26 changes: 25 additions & 1 deletion lib/project_types/theme/messages/messages.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,36 @@ module Messages
{{command:--poll}} Force polling to detect file changes
{{command:--host=HOST}} Set which network interface the web server listens on. The default value is 127.0.0.1.
HELP
serve: "Viewing theme…",
viewing_theme: "Viewing theme…",
syncing_theme: "Syncing theme #%s on %s",
open_fail: "Couldn't open the theme",
error: {
address_binding_error: "Couldn't bind to localhost."\
" To serve your theme, set a different address with {{command:%s theme serve --host=<address>}}",
},
serving: <<~SERVING,
Serving %s
SERVING
customize_or_preview: <<~CUSTOMIZE_OR_PREVIEW,
Customize this theme in the Online Store Editor:
{{green:%s}}
Share this theme preview:
{{green:%s}}
(Use Ctrl-C to stop)
CUSTOMIZE_OR_PREVIEW
ensure_user: <<~ENSURE_USER,
You are not authorized to edit themes on %s.
Make sure you are a user of that store, and allowed to edit themes.
ENSURE_USER
already_in_use_error: "Error",
address_already_in_use: "The address \"%s\" is already in use.",
try_this: "Try this",
try_port_option: "Use the --port=PORT option to serve the theme in a different port.",
},
check: {
help: <<~HELP,
Expand Down
43 changes: 27 additions & 16 deletions lib/shopify_cli/theme/dev_server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ def start(ctx, root, http_bind: "127.0.0.1", port: 9292, poll: false)
@app = LocalAssets.new(ctx, @app, theme: theme)
@app = HotReload.new(ctx, @app, theme: theme, watcher: watcher, ignore_filter: ignore_filter)
stopped = false
address = "http://#{http_bind}:#{port}"

theme.ensure_exists!

Expand All @@ -44,8 +45,8 @@ def start(ctx, root, http_bind: "127.0.0.1", port: 9292, poll: false)
stop
end

CLI::UI::Frame.open(@ctx.message("theme.serve.serve")) do
ctx.print_task("Syncing theme ##{theme.id} on #{theme.shop}")
CLI::UI::Frame.open(@ctx.message("theme.serve.viewing_theme")) do
ctx.print_task(ctx.message("theme.serve.syncing_theme", theme.id, theme.shop))
@syncer.start_threads
if block_given?
yield @syncer
Expand All @@ -55,18 +56,9 @@ def start(ctx, root, http_bind: "127.0.0.1", port: 9292, poll: false)

return if stopped

ctx.puts("")
ctx.puts("Serving #{theme.root}")
ctx.puts("")
ctx.open_url!("http://127.0.0.1:#{port}")
ctx.puts("")
ctx.puts("Customize this theme in the Online Store Editor:")
ctx.puts("{{green:#{theme.editor_url}}}")
ctx.puts("")
ctx.puts("Share this theme preview:")
ctx.puts("{{green:#{theme.preview_url}}}")
ctx.puts("")
ctx.puts("(Use Ctrl-C to stop)")
ctx.puts(ctx.message("theme.serve.serving", theme.root))
ctx.open_url!(address)
ctx.puts(ctx.message("theme.serve.customize_or_preview", theme.editor_url, theme.preview_url))
end

logger = if ctx.debug?
Expand All @@ -87,8 +79,9 @@ def start(ctx, root, http_bind: "127.0.0.1", port: 9292, poll: false)

rescue ShopifyCLI::API::APIRequestForbiddenError,
ShopifyCLI::API::APIRequestUnauthorizedError
@ctx.abort("You are not authorized to edit themes on #{theme.shop}.\n" \
"Make sure you are a user of that store, and allowed to edit themes.")
raise ShopifyCLI::Abort, @ctx.message("theme.serve.ensure_user", theme.shop)
rescue Errno::EADDRINUSE
abort_address_already_in_use(address)
rescue Errno::EADDRNOTAVAIL
raise AddressBindingError, "Error binding to the address #{http_bind}."
end
Expand All @@ -99,6 +92,24 @@ def stop
@syncer.shutdown
WebServer.shutdown
end

private

def abort_address_already_in_use(address)
open_frame(@ctx.message("theme.serve.already_in_use_error"), color: :red) do
@ctx.puts(@ctx.message("theme.serve.address_already_in_use", address))
end

open_frame(@ctx.message("theme.serve.try_this"), color: :green) do
@ctx.puts(@ctx.message("theme.serve.try_port_option"))
end

raise ShopifyCLI::AbortSilent
end

def open_frame(title, color:, &block)
CLI::UI::Frame.open(title, color: CLI::UI.resolve_color(color), timing: false, &block)
end
end
end
end
Expand Down
64 changes: 31 additions & 33 deletions test/shopify-cli/theme/dev_server/integration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,7 @@ def test_proxy_to_sfr
end

def test_uploads_files_on_boot
# Get the checksums
stub_request(:any, ASSETS_API_URL)
.to_return(status: 200, body: "{}")
stub_request(:any, THEMES_API_URL)
.to_return(status: 200, body: "{}")

start_server
# Wait for server to start & sync the files
get("/assets/bogus.css")
start_server_and_wait_sync_files

# Should upload all theme files except the ignored files
ignored_files = [
Expand All @@ -99,15 +91,7 @@ def test_uploads_files_on_boot
end

def test_uploads_files_on_modification
# Get the checksums
stub_request(:any, ASSETS_API_URL)
.to_return(status: 200, body: "{}")
stub_request(:any, THEMES_API_URL)
.to_return(status: 200, body: "{}")

start_server
# Wait for server to start & sync the files
get("/assets/bogus.css")
start_server_and_wait_sync_files

theme_root = "#{ShopifyCLI::ROOT}/test/fixtures/theme"

Expand All @@ -131,26 +115,28 @@ def test_uploads_files_on_modification
end

def test_serve_assets_locally
stub_request(:any, ASSETS_API_URL)
.to_return(status: 200, body: "{}")
stub_request(:any, THEMES_API_URL)
.to_return(status: 200, body: "{}")

start_server
response = get("/assets/theme.css")
response = start_server_and_wait_sync_files

refute_server_errors(response)
end

def test_streams_hot_reload_events
stub_request(:any, ASSETS_API_URL)
.to_return(status: 200, body: "{}")
stub_request(:any, THEMES_API_URL)
.to_return(status: 200, body: "{}")
def test_address_already_in_use
start_server_and_wait_sync_files

start_server
# Wait for server to start
get("/assets/theme.css")
@ctx.output_captured = true
io = capture_io_and_assert_raises(ShopifyCLI::AbortSilent) do
DevServer.start(@ctx, "#{ShopifyCLI::ROOT}/test/fixtures/theme", port: @@port)
end
@ctx.output_captured = false

io_messages = io.join

assert_match @ctx.message("theme.serve.address_already_in_use", "http://127.0.0.1:#{@@port}"), io_messages
assert_match @ctx.message("theme.serve.try_port_option"), io_messages
end

def test_streams_hot_reload_events
start_server_and_wait_sync_files

# Send the SSE request
socket = TCPSocket.new("127.0.0.1", @@port)
Expand Down Expand Up @@ -184,6 +170,18 @@ def start_server
end
end

def start_server_and_wait_sync_files
# Get the checksums
stub_request(:any, ASSETS_API_URL)
.to_return(status: 200, body: "{}")
stub_request(:any, THEMES_API_URL)
.to_return(status: 200, body: "{}")

start_server
# Wait for server to start & sync the files
get("/assets/bogus.css")
end

def refute_server_errors(response)
refute_match(/error/i, response, response)
end
Expand Down

0 comments on commit 3e7c66d

Please sign in to comment.