fix(dev): login and logout commands didn't work in dev command

Also added a timeout for the login command.

core/src/cli/cli.ts

@@ -298,7 +298,7 @@ ${renderCommands(commands)}
     if (!command.noProject) {
       const config: ProjectResource | undefined = await this.getProjectConfig(log, workingDir)
 
-      const cloudDomain = getGardenCloudDomain(config)
+      const cloudDomain = getGardenCloudDomain(config?.domain)
       const distroName = getCloudDistributionName(cloudDomain)
 
       try {

core/src/cli/command-line.ts

@@ -563,7 +563,8 @@ ${renderDivider({ width, char, color })}
           })
         }
       })
-      .catch(() => {
+      .catch((error: Error) => {
+        this.log.error({ error })
         this.log.error({ msg: renderDivider({ width, color: chalk.red, char: "┈" }) })
         this.flashError(failMessage)
       })

core/src/cloud/api.ts

@@ -27,7 +27,6 @@ import { CommandInfo } from "../plugin-context"
 import { ClientAuthToken, GlobalConfigStore } from "../config-store/global"
 import { add } from "date-fns"
 import { LogLevel } from "../logger/logger"
-import { ProjectResource } from "../config/project"
 
 const gardenClientName = "garden-core"
 const gardenClientVersion = getPackageVersion()
@@ -139,13 +138,13 @@ function toCloudProject(
  * A helper function to get the cloud domain from a project config. Uses the env var
  * GARDEN_CLOUD_DOMAIN to override a configured domain.
  */
-export function getGardenCloudDomain(projectConfig?: ProjectResource): string {
+export function getGardenCloudDomain(configuredDomain: string | undefined): string {
   let cloudDomain: string | undefined
 
   if (gardenEnv.GARDEN_CLOUD_DOMAIN) {
     cloudDomain = new URL(gardenEnv.GARDEN_CLOUD_DOMAIN).origin
-  } else if (projectConfig?.domain) {
-    cloudDomain = new URL(projectConfig.domain).origin
+  } else if (configuredDomain) {
+    cloudDomain = new URL(configuredDomain).origin
   }
 
   return cloudDomain || DEFAULT_GARDEN_CLOUD_DOMAIN

core/src/commands/login.ts

@@ -11,11 +11,12 @@ import { printHeader } from "../logger/util"
 import dedent = require("dedent")
 import { AuthTokenResponse, CloudApi, getGardenCloudDomain } from "../cloud/api"
 import { Log } from "../logger/log-entry"
-import { ConfigurationError, InternalError } from "../exceptions"
+import { ConfigurationError, InternalError, TimeoutError } from "../exceptions"
 import { AuthRedirectServer } from "../cloud/auth"
 import { EventBus } from "../events"
 import { getCloudDistributionName } from "../util/util"
-import { ProjectResource } from "../config/project"
+
+const loginTimeoutSec = 60
 
 export class LoginCommand extends Command {
   name = "login"
@@ -40,16 +41,21 @@ export class LoginCommand extends Command {
     // The Enterprise API is missing from the Garden class for commands with noProject
     // so we initialize it here.
     const globalConfigStore = garden.globalConfigStore
-    const projectConfig: ProjectResource | undefined = await cli!.getProjectConfig(log, garden.projectRoot)
-
-    // Fail if this is not run within a garden project
-    if (!projectConfig) {
-      throw new ConfigurationError(
-        `Not a project directory (or any of the parent directories): ${garden.projectRoot}`,
-        {
-          root: garden.projectRoot,
-        }
-      )
+
+    let configuredDomain = garden.cloudDomain
+
+    if (!configuredDomain) {
+      const projectConfig = await cli?.getProjectConfig(log, garden.projectRoot)
+
+      // Fail if this is not run within a garden project
+      if (!projectConfig) {
+        throw new ConfigurationError(
+          `Not a project directory (or any of the parent directories): ${garden.projectRoot}`,
+          {
+            root: garden.projectRoot,
+          }
+        )
+      }
     }
 
     // Garden works by default without Garden Cloud. In order to use cloud, a domain
@@ -62,7 +68,7 @@ export class LoginCommand extends Command {
     //
     // If the fallback was used, we rely on the token to decide if the Cloud API instance
     // should use the default domain or not. The token lifecycle ends on logout.
-    let cloudDomain: string = getGardenCloudDomain(projectConfig)
+    let cloudDomain: string = getGardenCloudDomain(configuredDomain)
 
     const distroName = getCloudDistributionName(cloudDomain)
 
@@ -100,10 +106,22 @@ export async function login(log: Log, cloudDomain: string, events: EventBus) {
   const server = new AuthRedirectServer(cloudDomain, events, log)
   const distroName = getCloudDistributionName(cloudDomain)
   log.debug(`Redirecting to ${distroName} login page...`)
-  const response: AuthTokenResponse = await new Promise(async (resolve, _reject) => {
+  const response: AuthTokenResponse = await new Promise(async (resolve, reject) => {
     // The server resolves the promise with the new auth token once it's received the redirect.
     await server.start()
+
+    let timedOut = false
+
+    const timeout = setTimeout(() => {
+      timedOut = true
+      reject(new TimeoutError(`Timed out after ${loginTimeoutSec} seconds, waiting for web login response.`, {}))
+    }, loginTimeoutSec * 1000)
+
     events.once("receivedToken", (tokenResponse: AuthTokenResponse) => {
+      if (timedOut) {
+        return
+      }
+      clearTimeout(timeout)
       log.debug("Received client auth token.")
       resolve(tokenResponse)
     })

core/src/commands/logout.ts

@@ -11,7 +11,6 @@ import { printHeader } from "../logger/util"
 import { CloudApi, getGardenCloudDomain } from "../cloud/api"
 import { dedent } from "../util/string"
 import { getCloudDistributionName } from "../util/util"
-import { ProjectResource } from "../config/project"
 import { ConfigurationError } from "../exceptions"
 
 export class LogOutCommand extends Command {
@@ -31,19 +30,24 @@ export class LogOutCommand extends Command {
   async action({ cli, garden, log }: CommandParams): Promise<CommandResult> {
     // The Enterprise API is missing from the Garden class for commands with noProject
     // so we initialize it here.
-    const projectConfig: ProjectResource | undefined = await cli!.getProjectConfig(log, garden.projectRoot)
-    const cloudDomain: string | undefined = getGardenCloudDomain(projectConfig)
+    let configuredDomain = garden.cloudDomain
 
-    // Fail if this is not run within a garden project
-    if (!projectConfig) {
-      throw new ConfigurationError(
-        `Not a project directory (or any of the parent directories): ${garden.projectRoot}`,
-        {
-          root: garden.projectRoot,
-        }
-      )
+    if (!configuredDomain) {
+      const projectConfig = await cli?.getProjectConfig(log, garden.projectRoot)
+
+      // Fail if this is not run within a garden project
+      if (!projectConfig) {
+        throw new ConfigurationError(
+          `Not a project directory (or any of the parent directories): ${garden.projectRoot}`,
+          {
+            root: garden.projectRoot,
+          }
+        )
+      }
     }
 
+    const cloudDomain: string | undefined = getGardenCloudDomain(configuredDomain)
+
     const distroName = getCloudDistributionName(cloudDomain)
 
     try {

core/src/exceptions.ts

@@ -11,7 +11,7 @@ import { stringify } from "yaml"
 import { withoutInternalFields, sanitizeValue } from "./util/logging"
 import { testFlags } from "./util/util"
 
-export interface GardenError<D extends object = any> {
+export interface GardenError<D extends object = any> extends Error {
   type: string
   message: string
   detail?: D

core/src/garden.ts

@@ -1581,7 +1581,7 @@ export const resolveGardenParams = profileAsync(async function _resolveGardenPar
   const cloudApi = opts.cloudApi || null
   // fall back to get the domain from config if the cloudApi instance failed
   // to login or was not defined.
-  const cloudDomain = cloudApi?.domain || getGardenCloudDomain(config)
+  const cloudDomain = cloudApi?.domain || getGardenCloudDomain(config.domain)
 
   // The cloudApi instance only has a project ID when the configured ID has
   // been verified against the cloud instance.

core/src/logger/log-entry.ts

@@ -12,7 +12,6 @@ import { cloneDeep, round } from "lodash"
 import { LogLevel, logLevelMap } from "./logger"
 import { Omit } from "../util/util"
 import { findParentLogContext } from "./util"
-import { GardenError } from "../exceptions"
 import { Logger } from "./logger"
 import uniqid from "uniqid"
 
@@ -53,7 +52,7 @@ interface LogParams extends LogCommonParams {
   symbol?: LogSymbol
   data?: any
   dataFormat?: "json" | "yaml"
-  error?: GardenError
+  error?: Error
 }
 
 interface CreateLogEntryParams extends LogParams {

core/src/logger/renderers.ts

@@ -17,7 +17,7 @@ import { LogEntry } from "./log-entry"
 import { JsonLogEntry } from "./writers/json-terminal-writer"
 import { highlightYaml, safeDumpYaml } from "../util/serialization"
 import { Logger, logLevelMap, LogLevel } from "./logger"
-import { formatGardenErrorWithDetail } from "../exceptions"
+import { toGardenError, formatGardenErrorWithDetail } from "../exceptions"
 
 type RenderFn = (entry: LogEntry) => string
 
@@ -45,7 +45,7 @@ export function combineRenders(entry: LogEntry, renderers: RenderFn[]): string {
 export function renderError(entry: LogEntry): string {
   const { error } = entry
   if (error) {
-    return formatGardenErrorWithDetail(error)
+    return formatGardenErrorWithDetail(toGardenError(error))
   }
 
   return entry.msg || ""
@@ -138,14 +138,21 @@ export function renderSection(entry: LogEntry): string {
  * Formats entries for the terminal writer.
  */
 export function formatForTerminal(entry: LogEntry): string {
-  const { msg: msg, section, symbol, data } = entry
+  const { msg, section, symbol, data, error } = entry
   const empty = [msg, section, symbol, data].every((val) => val === undefined)
 
   if (empty) {
     return ""
   }
 
-  return combineRenders(entry, [renderTimestamp, renderSymbol, renderSection, renderMsg, renderData, () => "\n"])
+  let out = combineRenders(entry, [renderTimestamp, renderSymbol, renderSection, renderMsg, renderData])
+
+  // If no data or msg is set, only error, render the error (which is normally only sent to error log file)
+  if (error && !data && !msg) {
+    out += renderError(entry)
+  }
+
+  return out + "\n"
 }
 
 export function cleanForJSON(input?: string | string[]): string {
@@ -171,7 +178,7 @@ export function render(entry: LogEntry, logger: Logger): string | null {
 // TODO: Include individual message states with timestamp
 export function formatForJson(entry: LogEntry): JsonLogEntry {
   const { msg, metadata, section } = entry
-  const errorDetail = entry.error && entry ? formatGardenErrorWithDetail(entry.error) : undefined
+  const errorDetail = entry.error && entry ? formatGardenErrorWithDetail(toGardenError(entry.error)) : undefined
   const jsonLogEntry: JsonLogEntry = {
     msg: cleanForJSON(msg),
     data: entry.data,

core/test/unit/src/commands/login.ts

@@ -150,12 +150,16 @@ describe("LoginCommand", () => {
       globalConfigStore,
     })
 
+    // Need to override the default because we're using DummyGarden
+    const cloudDomain = "http://example.invalid"
+    Object.assign(garden, { cloudDomain })
+
     setTimeout(() => {
       garden.events.emit("receivedToken", testToken)
     }, 500)
 
     await command.action(makeCommandParams({ cli, garden, args: {}, opts: {} }))
-    const savedToken = await CloudApi.getStoredAuthToken(garden.log, garden.globalConfigStore, "http://example.invalid")
+    const savedToken = await CloudApi.getStoredAuthToken(garden.log, garden.globalConfigStore, cloudDomain)
     expect(savedToken).to.exist
     expect(savedToken!.token).to.eql(testToken.token)
     expect(savedToken!.refreshToken).to.eql(testToken.refreshToken)

core/test/unit/src/logger/renderers.ts

@@ -50,6 +50,7 @@ describe("renderers", () => {
   describe("renderError", () => {
     it("should render error object if present", () => {
       const error: GardenError = {
+        name: "test",
         message: "hello error",
         type: "a",
         detail: {
@@ -58,13 +59,9 @@ describe("renderers", () => {
         },
       }
       const log = logger.makeNewLogContext().info({ msg: "foo", error })
-      expect(renderError(log.entries[0])).to.equal(dedent`
-          hello error
-
-          Error Details:
-
-          foo: bar\n
-        `)
+      const rendered = renderError(log.entries[0])
+      expect(rendered).to.include("Error: hello error")
+      expect(rendered).to.include("Error Details:")
     })
   })
   describe("renderSection", () => {