builder: Use "failed" on failure to ensure front-end indicates failure

Converts `ProgressEvent.BuildStatus` into a string Enum, and uses Enum values to ensure consistent use of phases. Note repo2docker uses `"failure"` instead of `"failed"` so the event source still has a mix of failed/failure

Author: manics

binderhub/build.py

@@ -40,13 +40,16 @@ class BuildStatus(Enum):
         The state the build is now in
 
         Used when `kind` is `Kind.BUILD_STATUS_CHANGE`
+
+        These enum values are referenced in the front-end to display the build status,
+        hence the mismatch between the Enum name (backend) and value (frontend).
         """
 
-        PENDING = 1
-        RUNNING = 2
-        COMPLETED = 3
-        FAILED = 4
-        UNKNOWN = 5
+        PENDING = "pending"
+        RUNNING = "running"
+        COMPLETED = "built"
+        FAILED = "failed"
+        UNKNOWN = "unknown"
 
     def __init__(self, kind: Kind, payload: Union[str, BuildStatus]):
         self.kind = kind

binderhub/builder.py

@@ -509,12 +509,13 @@ def _check_result(future):
                 # FIXME: If pod goes into an unrecoverable stage, such as ImagePullBackoff or
                 # whatever, we should fail properly.
                 if progress.kind == ProgressEvent.Kind.BUILD_STATUS_CHANGE:
+                    phase = progress.payload.value
                     if progress.payload == ProgressEvent.BuildStatus.PENDING:
                         # nothing to do, just waiting
                         continue
                     elif progress.payload == ProgressEvent.BuildStatus.COMPLETED:
                         event = {
-                            "phase": "built",
+                            "phase": phase,
                             "message": "Built image, launching...\n",
                             "imageName": image_name,
                         }
@@ -529,9 +530,13 @@ def _check_result(future):
                         # Do nothing, is ok!
                         continue
                     elif progress.payload == ProgressEvent.BuildStatus.FAILED:
-                        event = {"phase": "failure"}
+                        event = {"phase": phase}
                     elif progress.payload == ProgressEvent.BuildStatus.UNKNOWN:
-                        event = {"phase": "unknown"}
+                        event = {"phase": phase}
+                    else:
+                        raise ValueError(
+                            f"Found unknown phase {phase} in ProgressEvent"
+                        )
                 elif progress.kind == ProgressEvent.Kind.LOG_MESSAGE:
                     # The logs are coming out of repo2docker, so we expect
                     # them to be JSON structured anyway

binderhub/tests/test_build.py

@@ -94,6 +94,30 @@ async def test_build(app, needs_build, needs_launch, always_build, slug, pytestc
     assert r.url.startswith(final["url"])
 
 
+@pytest.mark.asyncio(timeout=120)
+@pytest.mark.remote
+async def test_build_fail(app, needs_build, needs_launch, always_build, pytestconfig):
+    """
+    Test build a repo that should fail immediately.
+    """
+    slug = "gist/manics/cc003c8264db20d22b5fa437770f655e"
+    build_url = f"{app.url}/build/{slug}"
+    r = await async_requests.get(build_url, stream=True)
+    r.raise_for_status()
+    failed_events = 0
+    async for line in async_requests.iter_lines(r):
+        line = line.decode("utf8", "replace")
+        if line.startswith("data:"):
+            event = json.loads(line.split(":", 1)[1])
+            assert event.get("phase") not in ("launching", "ready")
+            if event.get("phase") == "failed":
+                failed_events += 1
+                break
+    r.close()
+
+    assert failed_events > 0, "Should have seen phase 'failed'"
+
+
 def _list_dind_pods_mock():
     """Mock list of DIND pods"""
     mock_response = mock.MagicMock()