Skip to content

Commit

Permalink
Improve extension download error in install report (#684)
Browse files Browse the repository at this point in the history
When the extension installer can't download from any of the mirrors,
make the error that's written to the installation report more clear.

Previously the cloudfront mirror download would report `{:error,
{:error, :closed}}` as the installation failure reason, but this isn't
very descriptive.

I've wrapped the error in a more descriptive message so we can see where
the error actually comes from and what's happening. This format is
similar to how our Ruby gem formats this type of error:
https://github.com/appsignal/appsignal-ruby/blob/409fbf98e3974317243ae85a6f2e6e77132db88b/ext/base.rb#L142-L151

As far as I know the cloudfront mirror will always fail currently, so
this only happens if the fastly mirror (the first one it tries), also
fails.

Part of #683
  • Loading branch information
tombruijn authored Oct 7, 2021
1 parent f9d9d23 commit 787684b
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changesets/better-download-error-reporting.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
bump: "patch"
---

Installation report improved for download errors. Download errors are more descriptive in the installation result of the diagnose report.
35 changes: 30 additions & 5 deletions mix_helpers.exs
Original file line number Diff line number Diff line change
Expand Up @@ -173,15 +173,34 @@ defmodule Mix.Appsignal.Helper do
end

defp do_download_file!(filename, local_filename, mirrors) do
Enum.reduce_while(mirrors, 1, fn mirror, acc ->
Enum.reduce_while(mirrors, {1, []}, fn mirror, {acc, errors} ->
version = Appsignal.Agent.version()
url = build_download_url(mirror, version, filename)
result = do_download_file!(url, local_filename)

if result == :ok or length(mirrors) == acc do
{:halt, {result, url}}
if result == :ok do
# Success on download
{:halt, {:ok, url}}
else
{:cont, acc + 1}
if length(mirrors) == acc do
# All mirrors failed. Write error message detailing all failures
error_messages =
Enum.map(Enum.reverse([result | errors]), fn {:error, message} ->
message
end)

message = """
Could not download archive from any of our mirrors.
Please make sure your network allows access to any of these mirrors.
Attempted to download the archive from the following urls:
#{Enum.join(error_messages, "\n")}
"""

{:halt, {String.trim(message), url}}
else
# Try the next mirror
{:cont, {acc + 1, [result | errors]}}
end
end
end)
end
Expand All @@ -195,7 +214,13 @@ defmodule Mix.Appsignal.Helper do
end

response ->
{:error, response}
message = """
- URL: #{url}
- Error (hackney response):
#{inspect(response)}
"""

{:error, message}
end
end

Expand Down

0 comments on commit 787684b

Please sign in to comment.