From 740b0bf6316dc9a99cdf905323e9a9c3944e5120 Mon Sep 17 00:00:00 2001
From: Thomas Sibley <tsibley@fredhutch.org>
Date: Tue, 20 Aug 2024 14:36:14 -0700
Subject: [PATCH] remote.nextstrain_dot_org: Include an explicit origin in
 error messages
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Otherwise, for remote origins other than the default of nextstrain.org,
the error messages suggest incorrect/misleading possible solutions in
the form of commands to run.

Motivated by @jameshadfield's comments in Slack.¹

¹ <https://bedfordlab.slack.com/archives/C01LCTT7JNN/p1723760025178909?thread_ts=1723701856.612259&cid=C01LCTT7JNN>
---
 CHANGES.md                                  |  9 ++++++
 nextstrain/cli/remote/nextstrain_dot_org.py | 36 ++++++++++++++-------
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index 6dda77cd..b6a79723 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -13,6 +13,15 @@ development source code and as such may not be routinely kept up to date.
 
 # __NEXT__
 
+## Bug fixes
+
+* The suggested commands to run (i.e. potential solutions) in expected errors
+  from `nextstrain remote` now explicitly include the remote origin to avoid
+  being incorrect or misleading for origins other than nextstrain.org.  For
+  example, if the error message suggested running `nextstrain login`, it now
+  suggests `nextstrain login https://nextstrain.org`.
+  ([#391](https://github.com/nextstrain/cli/pull/391))
+
 
 # 8.5.1 (31 July 2024)
 
diff --git a/nextstrain/cli/remote/nextstrain_dot_org.py b/nextstrain/cli/remote/nextstrain_dot_org.py
index c57c83d9..08a2a7e3 100644
--- a/nextstrain/cli/remote/nextstrain_dot_org.py
+++ b/nextstrain/cli/remote/nextstrain_dot_org.py
@@ -69,6 +69,7 @@
 from email.message import EmailMessage
 from pathlib import Path, PurePosixPath
 from requests.utils import parse_dict_header
+from shlex import quote as shquote
 from tempfile import NamedTemporaryFile
 from textwrap import indent, wrap
 from typing import Dict, Iterable, List, NamedTuple, Optional, Tuple, Union
@@ -264,7 +265,7 @@ def put(endpoint, file, media_type):
                     else:
                         raise
 
-                raise_for_status(response)
+                raise_for_status(origin, response)
 
         # Upload datasets
         for dataset, files in datasets.items():
@@ -377,7 +378,7 @@ def download(url: URL, local_path: Path, recursively: bool = False, dry_run: boo
                         continue
 
                     # Check for bad response
-                    raise_for_status(response)
+                    raise_for_status(origin, response)
 
                     if content_media_type(response) != subresource.media_type:
                         raise UserError(f"Path {path} does not seem to be a {subresource}.")
@@ -540,7 +541,7 @@ def _ls(origin: Origin, path: NormalizedPath, recursively: bool = False, http: r
         params = {"prefix": str(path)},
         headers = {"Accept": "application/json"})
 
-    raise_for_status(response)
+    raise_for_status(origin, response)
 
     available = response.json()
 
@@ -613,7 +614,7 @@ def delete(url: URL, recursively: bool = False, dry_run: bool = False) -> Iterab
 
             response = http.delete(endpoint)
 
-            raise_for_status(response)
+            raise_for_status(origin, response)
 
             assert response.status_code == 204
 
@@ -807,7 +808,7 @@ def __call__(self, request: requests.PreparedRequest) -> requests.PreparedReques
         return request
 
 
-def raise_for_status(response: requests.Response) -> None:
+def raise_for_status(origin: Origin, response: requests.Response) -> None:
     """
     Human-centered error handling for nextstrain.org API responses.
 
@@ -875,26 +876,39 @@ def raise_for_status(response: requests.Response) -> None:
                     # enough to handle things like re-seeking streams (which
                     # may not be possible without cooperation).
                     #   -trs, 10 May 2022
-                    raise UserError("""
+                    raise UserError(f"""
                         Login credentials appear to be out of date.
 
-                        Please run `nextstrain login --renew` and then retry your command.
+                        Please run
+
+                            nextstrain login --renew {shquote(origin)}
+
+                        and then retry your command.
                         """) from err
                 else:
                     raise UserError(f"""
                         Permission denied.
 
-                        Are you logged in as the correct user?  Current user: {user.username}.
+                        Are you logged in as the correct user?
+
+                        Current user: {user.username}
 
                         If your permissions were recently changed (e.g. new group
-                        membership), it might help to run `nextstrain login --renew`
+                        membership), it might help to run
+
+                            nextstrain login --renew {shquote(origin)}
+
                         and then retry your command.
                         """) from err
             else:
-                raise UserError("""
+                raise UserError(f"""
                     Permission denied.
 
-                    Logging in with `nextstrain login` might help?
+                    Logging in with
+
+                        nextstrain login {shquote(origin)}
+
+                    might help?
                     """) from err
 
         elif status == 404: