feat(client): improve logging

README.md

@@ -187,6 +187,59 @@ console.log(raw.headers.get('X-My-Header'));
 console.log(response.organizationId);
 ```
 
+### Logging
+
+> [!IMPORTANT]
+> All log messages are intended for debugging only. The format and content of log messages
+> may change between releases.
+
+#### Log levels
+
+The log level can be configured in two ways:
+
+1. Via the `GITPOD_LOG` environment variable
+2. Using the `logLevel` client option (overrides the environment variable if set)
+
+```ts
+import Gitpod from '@gitpod/sdk';
+
+const client = new Gitpod({
+  logLevel: 'debug', // Show all log messages
+});
+```
+
+Available log levels, from most to least verbose:
+
+- `'debug'` - Show debug messages, info, warnings, and errors
+- `'info'` - Show info messages, warnings, and errors
+- `'warn'` - Show warnings and errors (default)
+- `'error'` - Show only errors
+- `'off'` - Disable all logging
+
+At the `'debug'` level, all HTTP requests and responses are logged, including headers and bodies.
+Some authentication-related headers are redacted, but sensitive data in request and response bodies
+may still be visible.
+
+#### Custom logger
+
+By default, this library logs to `globalThis.console`. You can also provide a custom logger.
+Most logging libraries are supported, including [pino](https://www.npmjs.com/package/pino), [winston](https://www.npmjs.com/package/winston), [bunyan](https://www.npmjs.com/package/bunyan), [consola](https://www.npmjs.com/package/consola), [signale](https://www.npmjs.com/package/signale), and [@std/log](https://jsr.io/@std/log). If your logger doesn't work, please open an issue.
+
+When providing a custom logger, the `logLevel` option still controls which messages are emitted, messages
+below the configured level will not be sent to your logger.
+
+```ts
+import Gitpod from '@gitpod/sdk';
+import pino from 'pino';
+
+const logger = pino();
+
+const client = new Gitpod({
+  logger: logger.child({ name: 'Gitpod' }),
+  logLevel: 'debug', // Send all messages to pino, allowing it to filter
+});
+```
+
 ### Making custom/undocumented requests
 
 This library is typed for convenient access to the documented API. If you need to access undocumented
@@ -246,33 +299,12 @@ globalThis.fetch = fetch;
 Or pass it to the client:
 
 ```ts
+import Gitpod from '@gitpod/sdk';
 import fetch from 'my-fetch';
 
 const client = new Gitpod({ fetch });
 ```
 
-### Logging and middleware
-
-You may also provide a custom `fetch` function when instantiating the client,
-which can be used to inspect or alter the `Request` or `Response` before/after each request:
-
-```ts
-import { fetch } from 'undici'; // as one example
-import Gitpod from '@gitpod/sdk';
-
-const client = new Gitpod({
-  fetch: async (url: RequestInfo, init?: RequestInit): Promise<Response> => {
-    console.log('About to make a request', url, init);
-    const response = await fetch(url, init);
-    console.log('Got response', response);
-    return response;
-  },
-});
-```
-
-Note that if given a `GITPOD_LOG=debug` environment variable, this library will log all requests and responses automatically.
-This is intended for debugging purposes only and may change in the future without notice.
-
 ### Fetch options
 
 If you want to set custom `fetch` options without overriding the `fetch` function, you can provide a `fetchOptions` object when instantiating the client or making a request. (Request-specific options override client options.)

src/client.ts

@@ -3,7 +3,7 @@
 import type { RequestInit, RequestInfo, BodyInit } from './internal/builtin-types';
 import type { HTTPMethod, PromiseOrValue, MergedRequestInit } from './internal/types';
 import { uuid4 } from './internal/utils/uuid';
-import { validatePositiveInteger, isAbsoluteURL } from './internal/utils/values';
+import { validatePositiveInteger, isAbsoluteURL, hasOwn } from './internal/utils/values';
 import { sleep } from './internal/utils/sleep';
 import { castToError, isAbortError } from './internal/errors';
 import type { APIResponseProps } from './internal/parse';
@@ -122,7 +122,7 @@ import {
   SecretsSecretsPage,
 } from './resources/secrets';
 import { readEnv } from './internal/utils/env';
-import { logger } from './internal/utils/log';
+import { formatRequestDetails, loggerFor } from './internal/utils/log';
 import { isEmptyObj } from './internal/utils/values';
 import {
   AdmissionLevel,
@@ -251,15 +251,30 @@ export type Logger = {
   debug: LogFn;
 };
 export type LogLevel = 'off' | 'error' | 'warn' | 'info' | 'debug';
-const isLogLevel = (key: string | undefined): key is LogLevel => {
+const parseLogLevel = (
+  maybeLevel: string | undefined,
+  sourceName: string,
+  client: Gitpod,
+): LogLevel | undefined => {
+  if (!maybeLevel) {
+    return undefined;
+  }
   const levels: Record<LogLevel, true> = {
     off: true,
     error: true,
     warn: true,
     info: true,
     debug: true,
   };
-  return key! in levels;
+  if (hasOwn(levels, maybeLevel)) {
+    return maybeLevel;
+  }
+  loggerFor(client).warn(
+    `${sourceName} was set to ${JSON.stringify(maybeLevel)}, expected one of ${JSON.stringify(
+      Object.keys(levels),
+    )}`,
+  );
+  return undefined;
 };
 
 export interface ClientOptions {
@@ -323,16 +338,16 @@ export interface ClientOptions {
   /**
    * Set the log level.
    *
-   * Defaults to process.env['GITPOD_LOG'].
+   * Defaults to process.env['GITPOD_LOG'] or 'warn' if it isn't set.
    */
-  logLevel?: LogLevel | undefined | null;
+  logLevel?: LogLevel | undefined;
 
   /**
    * Set the logger.
    *
    * Defaults to globalThis.console.
    */
-  logger?: Logger | undefined | null;
+  logger?: Logger | undefined;
 }
 
 type FinalizedRequestInit = RequestInit & { headers: Headers };
@@ -387,14 +402,13 @@ export class Gitpod {
     this.baseURL = options.baseURL!;
     this.timeout = options.timeout ?? Gitpod.DEFAULT_TIMEOUT /* 1 minute */;
     this.logger = options.logger ?? console;
-    if (options.logLevel != null) {
-      this.logLevel = options.logLevel;
-    } else {
-      const envLevel = readEnv('GITPOD_LOG');
-      if (isLogLevel(envLevel)) {
-        this.logLevel = envLevel;
-      }
-    }
+    const defaultLogLevel = 'warn';
+    // Set default logLevel early so that we can log a warning in parseLogLevel.
+    this.logLevel = defaultLogLevel;
+    this.logLevel =
+      parseLogLevel(options.logLevel, 'ClientOptions.logLevel', this) ??
+      parseLogLevel(readEnv('GITPOD_LOG'), "process.env['GITPOD_LOG']", this) ??
+      defaultLogLevel;
     this.fetchOptions = options.fetchOptions;
     this.maxRetries = options.maxRetries ?? 2;
     this.fetch = options.fetch ?? Shims.getDefaultFetch();
@@ -524,12 +538,13 @@ export class Gitpod {
     options: PromiseOrValue<FinalRequestOptions>,
     remainingRetries: number | null = null,
   ): APIPromise<Rsp> {
-    return new APIPromise(this, this.makeRequest(options, remainingRetries));
+    return new APIPromise(this, this.makeRequest(options, remainingRetries, undefined));
   }
 
   private async makeRequest(
     optionsInput: PromiseOrValue<FinalRequestOptions>,
     retriesRemaining: number | null,
+    retryOfRequestLogID: string | undefined,
   ): Promise<APIResponseProps> {
     const options = await optionsInput;
     const maxRetries = options.maxRetries ?? this.maxRetries;
@@ -543,60 +558,142 @@ export class Gitpod {
 
     await this.prepareRequest(req, { url, options });
 
-    logger(this).debug('request', url, options, req.headers);
+    /** Not an API request ID, just for correlating local log entries. */
+    const requestLogID = 'log_' + ((Math.random() * (1 << 24)) | 0).toString(16).padStart(6, '0');
+    const retryLogStr = retryOfRequestLogID === undefined ? '' : `, retryOf: ${retryOfRequestLogID}`;
+    const startTime = Date.now();
+
+    loggerFor(this).debug(
+      `[${requestLogID}] sending request`,
+      formatRequestDetails({
+        retryOfRequestLogID,
+        method: options.method,
+        url,
+        options,
+        headers: req.headers,
+      }),
+    );
 
     if (options.signal?.aborted) {
       throw new Errors.APIUserAbortError();
     }
 
     const controller = new AbortController();
     const response = await this.fetchWithTimeout(url, req, timeout, controller).catch(castToError);
+    const headersTime = Date.now();
 
     if (response instanceof Error) {
+      const retryMessage = `retrying, ${retriesRemaining} attempts remaining`;
       if (options.signal?.aborted) {
         throw new Errors.APIUserAbortError();
       }
-      if (retriesRemaining) {
-        return this.retryRequest(options, retriesRemaining);
-      }
-      if (isAbortError(response)) {
-        throw new Errors.APIConnectionTimeoutError();
-      }
       // detect native connection timeout errors
       // deno throws "TypeError: error sending request for url (https://example/): client error (Connect): tcp connect error: Operation timed out (os error 60): Operation timed out (os error 60)"
       // undici throws "TypeError: fetch failed" with cause "ConnectTimeoutError: Connect Timeout Error (attempted address: example:443, timeout: 1ms)"
       // others do not provide enough information to distinguish timeouts from other connection errors
-      if (/timed? ?out/i.test(String(response) + ('cause' in response ? String(response.cause) : ''))) {
+      const isTimeout =
+        isAbortError(response) ||
+        /timed? ?out/i.test(String(response) + ('cause' in response ? String(response.cause) : ''));
+      if (retriesRemaining) {
+        loggerFor(this).info(
+          `[${requestLogID}] connection ${isTimeout ? 'timed out' : 'failed'} - ${retryMessage}`,
+        );
+        loggerFor(this).debug(
+          `[${requestLogID}] connection ${isTimeout ? 'timed out' : 'failed'} (${retryMessage})`,
+          formatRequestDetails({
+            retryOfRequestLogID,
+            url,
+            durationMs: headersTime - startTime,
+            message: response.message,
+          }),
+        );
+        return this.retryRequest(options, retriesRemaining, retryOfRequestLogID ?? requestLogID);
+      }
+      loggerFor(this).info(
+        `[${requestLogID}] connection ${isTimeout ? 'timed out' : 'failed'} - error; no more retries left`,
+      );
+      loggerFor(this).debug(
+        `[${requestLogID}] connection ${isTimeout ? 'timed out' : 'failed'} (error; no more retries left)`,
+        formatRequestDetails({
+          retryOfRequestLogID,
+          url,
+          durationMs: headersTime - startTime,
+          message: response.message,
+        }),
+      );
+      if (isTimeout) {
         throw new Errors.APIConnectionTimeoutError();
       }
       throw new Errors.APIConnectionError({ cause: response });
     }
 
+    const responseInfo = `[${requestLogID}${retryLogStr}] ${req.method} ${url} ${
+      response.ok ? 'succeeded' : 'failed'
+    } with status ${response.status} in ${headersTime - startTime}ms`;
+
     if (!response.ok) {
-      if (retriesRemaining && this.shouldRetry(response)) {
+      const shouldRetry = this.shouldRetry(response);
+      if (retriesRemaining && shouldRetry) {
         const retryMessage = `retrying, ${retriesRemaining} attempts remaining`;
-        logger(this).debug(`response (error; ${retryMessage})`, response.status, url, response.headers);
-        return this.retryRequest(options, retriesRemaining, response.headers);
+
+        // We don't need the body of this response.
+        await Shims.CancelReadableStream(response.body);
+        loggerFor(this).info(`${responseInfo} - ${retryMessage}`);
+        loggerFor(this).debug(
+          `[${requestLogID}] response error (${retryMessage})`,
+          formatRequestDetails({
+            retryOfRequestLogID,
+            url: response.url,
+            status: response.status,
+            headers: response.headers,
+            durationMs: headersTime - startTime,
+          }),
+        );
+        return this.retryRequest(
+          options,
+          retriesRemaining,
+          retryOfRequestLogID ?? requestLogID,
+          response.headers,
+        );
       }
 
+      const retryMessage = shouldRetry ? `error; no more retries left` : `error; not retryable`;
+
+      loggerFor(this).info(`${responseInfo} - ${retryMessage}`);
+
       const errText = await response.text().catch((err: any) => castToError(err).message);
       const errJSON = safeJSON(errText);
       const errMessage = errJSON ? undefined : errText;
-      const retryMessage = retriesRemaining ? `(error; no more retries left)` : `(error; not retryable)`;
 
-      logger(this).debug(
-        `response (error; ${retryMessage})`,
-        response.status,
-        url,
-        response.headers,
-        errMessage,
+      loggerFor(this).debug(
+        `[${requestLogID}] response error (${retryMessage})`,
+        formatRequestDetails({
+          retryOfRequestLogID,
+          url: response.url,
+          status: response.status,
+          headers: response.headers,
+          message: errMessage,
+          durationMs: Date.now() - startTime,
+        }),
       );
 
       const err = this.makeStatusError(response.status, errJSON, errMessage, response.headers);
       throw err;
     }
 
-    return { response, options, controller };
+    loggerFor(this).info(responseInfo);
+    loggerFor(this).debug(
+      `[${requestLogID}] response start`,
+      formatRequestDetails({
+        retryOfRequestLogID,
+        url: response.url,
+        status: response.status,
+        headers: response.headers,
+        durationMs: headersTime - startTime,
+      }),
+    );
+
+    return { response, options, controller, requestLogID, retryOfRequestLogID, startTime };
   }
 
   getAPIList<Item, PageClass extends Pagination.AbstractPage<Item> = Pagination.AbstractPage<Item>>(
@@ -614,7 +711,7 @@ export class Gitpod {
     Page: new (...args: ConstructorParameters<typeof Pagination.AbstractPage>) => PageClass,
     options: FinalRequestOptions,
   ): Pagination.PagePromise<PageClass, Item> {
-    const request = this.makeRequest(options, null);
+    const request = this.makeRequest(options, null, undefined);
     return new Pagination.PagePromise<PageClass, Item>(this as any as Gitpod, request, Page);
   }
 
@@ -677,6 +774,7 @@ export class Gitpod {
   private async retryRequest(
     options: FinalRequestOptions,
     retriesRemaining: number,
+    requestLogID: string,
     responseHeaders?: Headers | undefined,
   ): Promise<APIResponseProps> {
     let timeoutMillis: number | undefined;
@@ -709,7 +807,7 @@ export class Gitpod {
     }
     await sleep(timeoutMillis);
 
-    return this.makeRequest(options, retriesRemaining - 1);
+    return this.makeRequest(options, retriesRemaining - 1, requestLogID);
   }
 
   private calculateDefaultRetryTimeoutMillis(retriesRemaining: number, maxRetries: number): number {

src/internal/errors.ts

@@ -14,6 +14,17 @@ export function isAbortError(err: unknown) {
 export const castToError = (err: any): Error => {
   if (err instanceof Error) return err;
   if (typeof err === 'object' && err !== null) {
+    try {
+      if (Object.prototype.toString.call(err) === '[object Error]') {
+        // @ts-ignore - not all envs have native support for cause yet
+        const error = new Error(err.message, err.cause ? { cause: err.cause } : {});
+        if (err.stack) error.stack = err.stack;
+        // @ts-ignore - not all envs have native support for cause yet
+        if (err.cause && !error.cause) error.cause = err.cause;
+        if (err.name) error.name = err.name;
+        throw error;
+      }
+    } catch {}
     try {
       return new Error(JSON.stringify(err));
     } catch {}

src/internal/parse.ts

@@ -2,40 +2,51 @@
 
 import type { FinalRequestOptions } from './request-options';
 import { type Gitpod } from '../client';
-import { logger } from './utils/log';
+import { formatRequestDetails, loggerFor } from './utils/log';
 
 export type APIResponseProps = {
   response: Response;
   options: FinalRequestOptions;
   controller: AbortController;
+  requestLogID: string;
+  retryOfRequestLogID: string | undefined;
+  startTime: number;
 };
 
 export async function defaultParseResponse<T>(client: Gitpod, props: APIResponseProps): Promise<T> {
-  const { response } = props;
-
-  // fetch refuses to read the body when the status code is 204.
-  if (response.status === 204) {
-    return null as T;
-  }
-
-  if (props.options.__binaryResponse) {
-    return response as unknown as T;
-  }
-
-  const contentType = response.headers.get('content-type');
-  const isJSON =
-    contentType?.includes('application/json') || contentType?.includes('application/vnd.api+json');
-  if (isJSON) {
-    const json = await response.json();
-
-    logger(client).debug('response', response.status, response.url, response.headers, json);
-
-    return json as T;
-  }
-
-  const text = await response.text();
-  logger(client).debug('response', response.status, response.url, response.headers, text);
-
-  // TODO handle blob, arraybuffer, other content types, etc.
-  return text as unknown as T;
+  const { response, requestLogID, retryOfRequestLogID, startTime } = props;
+  const body = await (async () => {
+    // fetch refuses to read the body when the status code is 204.
+    if (response.status === 204) {
+      return null as T;
+    }
+
+    if (props.options.__binaryResponse) {
+      return response as unknown as T;
+    }
+
+    const contentType = response.headers.get('content-type');
+    const isJSON =
+      contentType?.includes('application/json') || contentType?.includes('application/vnd.api+json');
+    if (isJSON) {
+      const json = await response.json();
+      return json as T;
+    }
+
+    const text = await response.text();
+
+    // TODO handle blob, arraybuffer, other content types, etc.
+    return text as unknown as T;
+  })();
+  loggerFor(client).debug(
+    `[${requestLogID}] response parsed`,
+    formatRequestDetails({
+      retryOfRequestLogID,
+      url: response.url,
+      status: response.status,
+      body,
+      durationMs: Date.now() - startTime,
+    }),
+  );
+  return body;
 }

src/internal/shims.ts

@@ -143,3 +143,21 @@ export function ReadableStreamToAsyncIterable<T>(stream: any): AsyncIterableIter
     },
   };
 }
+
+/**
+ * Cancels a ReadableStream we don't need to consume.
+ * See https://undici.nodejs.org/#/?id=garbage-collection
+ */
+export async function CancelReadableStream(stream: any): Promise<void> {
+  if (stream === null || typeof stream !== 'object') return;
+
+  if (stream[Symbol.asyncIterator]) {
+    await stream[Symbol.asyncIterator]().return?.();
+    return;
+  }
+
+  const reader = stream.getReader();
+  const cancelPromise = reader.cancel();
+  reader.releaseLock();
+  await cancelPromise;
+}

src/internal/utils/log.ts

@@ -2,6 +2,7 @@
 
 import type { LogLevel, Logger } from '../../client';
 import { type Gitpod } from '../../client';
+import { RequestOptions } from '../request-options';
 
 const levelNumbers = {
   off: 0,
@@ -13,37 +14,85 @@ const levelNumbers = {
 
 function noop() {}
 
-function logFn(logger: Logger | undefined, clientLevel: LogLevel | undefined, level: keyof Logger) {
-  if (!logger || levelNumbers[level] > levelNumbers[clientLevel!]!) {
+function makeLogFn(fnLevel: keyof Logger, logger: Logger | undefined, logLevel: LogLevel) {
+  if (!logger || levelNumbers[fnLevel] > levelNumbers[logLevel]) {
     return noop;
   } else {
     // Don't wrap logger functions, we want the stacktrace intact!
-    return logger[level].bind(logger);
+    return logger[fnLevel].bind(logger);
   }
 }
 
-let lastLogger: { deref(): Logger } | undefined;
-let lastLevel: LogLevel | undefined;
-let lastLevelLogger: Logger;
+const noopLogger = {
+  error: noop,
+  warn: noop,
+  info: noop,
+  debug: noop,
+};
+
+let cachedLoggers = new WeakMap<Logger, [LogLevel, Logger]>();
+
+export function loggerFor(client: Gitpod): Logger {
+  const logger = client.logger;
+  const logLevel = client.logLevel ?? 'off';
+  if (!logger) {
+    return noopLogger;
+  }
 
-export function logger(client: Gitpod): Logger {
-  let { logger, logLevel: clientLevel } = client;
-  if (lastLevel === clientLevel && (logger === lastLogger || logger === lastLogger?.deref())) {
-    return lastLevelLogger;
+  const cachedLogger = cachedLoggers.get(logger);
+  if (cachedLogger && cachedLogger[0] === logLevel) {
+    return cachedLogger[1];
   }
+
   const levelLogger = {
-    error: logFn(logger, clientLevel, 'error'),
-    warn: logFn(logger, clientLevel, 'warn'),
-    info: logFn(logger, clientLevel, 'info'),
-    debug: logFn(logger, clientLevel, 'debug'),
+    error: makeLogFn('error', logger, logLevel),
+    warn: makeLogFn('warn', logger, logLevel),
+    info: makeLogFn('info', logger, logLevel),
+    debug: makeLogFn('debug', logger, logLevel),
   };
-  const { WeakRef } = globalThis as any;
-  lastLogger =
-    logger ?
-      WeakRef ? new WeakRef(logger)
-      : { deref: () => logger }
-    : undefined;
-  lastLevel = clientLevel;
-  lastLevelLogger = levelLogger;
+
+  cachedLoggers.set(logger, [logLevel, levelLogger]);
+
   return levelLogger;
 }
+
+export const formatRequestDetails = (details: {
+  options?: RequestOptions | undefined;
+  headers?: Headers | Record<string, string> | undefined;
+  retryOfRequestLogID?: string | undefined;
+  retryOf?: string | undefined;
+  url?: string | undefined;
+  status?: number | undefined;
+  method?: string | undefined;
+  durationMs?: number | undefined;
+  message?: unknown;
+  body?: unknown;
+}) => {
+  if (details.options) {
+    details.options = { ...details.options };
+    delete details.options['headers']; // redundant + leaks internals
+  }
+  if (details.headers) {
+    details.headers = Object.fromEntries(
+      (details.headers instanceof Headers ? [...details.headers] : Object.entries(details.headers)).map(
+        ([name, value]) => [
+          name,
+          (
+            name.toLowerCase() === 'authorization' ||
+            name.toLowerCase() === 'cookie' ||
+            name.toLowerCase() === 'set-cookie'
+          ) ?
+            '***'
+          : value,
+        ],
+      ),
+    );
+  }
+  if ('retryOfRequestLogID' in details) {
+    if (details.retryOfRequestLogID) {
+      details.retryOf = details.retryOfRequestLogID;
+    }
+    delete details.retryOfRequestLogID;
+  }
+  return details;
+};

src/internal/utils/values.ts

@@ -26,7 +26,7 @@ export function isEmptyObj(obj: Object | null | undefined): boolean {
 }
 
 // https://eslint.org/docs/latest/rules/no-prototype-builtins
-export function hasOwn(obj: Object, key: string): boolean {
+export function hasOwn<T extends object = object>(obj: T, key: PropertyKey): key is keyof T {
   return Object.prototype.hasOwnProperty.call(obj, key);
 }
 

tests/index.test.ts

@@ -13,8 +13,6 @@ describe('instantiate client', () => {
   beforeEach(() => {
     jest.resetModules();
     process.env = { ...env };
-
-    console.warn = jest.fn();
   });
 
   afterEach(() => {
@@ -52,16 +50,26 @@ describe('instantiate client', () => {
     });
   });
   describe('logging', () => {
-    afterEach(() => {
+    const env = process.env;
+
+    beforeEach(() => {
+      process.env = { ...env };
       process.env['GITPOD_LOG'] = undefined;
     });
 
+    afterEach(() => {
+      process.env = env;
+    });
+
     const forceAPIResponseForClient = async (client: Gitpod) => {
       await new APIPromise(
         client,
         Promise.resolve({
           response: new Response(),
           controller: new AbortController(),
+          requestLogID: 'log_000000',
+          retryOfRequestLogID: undefined,
+          startTime: Date.now(),
           options: {
             method: 'get',
             path: '/',
@@ -85,6 +93,11 @@ describe('instantiate client', () => {
       expect(debugMock).toHaveBeenCalled();
     });
 
+    test('default logLevel is warn', async () => {
+      const client = new Gitpod({ bearerToken: 'My Bearer Token' });
+      expect(client.logLevel).toBe('warn');
+    });
+
     test('debug logs are skipped when log level is info', async () => {
       const debugMock = jest.fn();
       const logger = {
@@ -111,11 +124,29 @@ describe('instantiate client', () => {
 
       process.env['GITPOD_LOG'] = 'debug';
       const client = new Gitpod({ logger: logger, bearerToken: 'My Bearer Token' });
+      expect(client.logLevel).toBe('debug');
 
       await forceAPIResponseForClient(client);
       expect(debugMock).toHaveBeenCalled();
     });
 
+    test('warn when env var level is invalid', async () => {
+      const warnMock = jest.fn();
+      const logger = {
+        debug: jest.fn(),
+        info: jest.fn(),
+        warn: warnMock,
+        error: jest.fn(),
+      };
+
+      process.env['GITPOD_LOG'] = 'not a log level';
+      const client = new Gitpod({ logger: logger, bearerToken: 'My Bearer Token' });
+      expect(client.logLevel).toBe('warn');
+      expect(warnMock).toHaveBeenCalledWith(
+        'process.env[\'GITPOD_LOG\'] was set to "not a log level", expected one of ["off","error","warn","info","debug"]',
+      );
+    });
+
     test('client log level overrides env var', async () => {
       const debugMock = jest.fn();
       const logger = {
@@ -131,6 +162,21 @@ describe('instantiate client', () => {
       await forceAPIResponseForClient(client);
       expect(debugMock).not.toHaveBeenCalled();
     });
+
+    test('no warning logged for invalid env var level + valid client level', async () => {
+      const warnMock = jest.fn();
+      const logger = {
+        debug: jest.fn(),
+        info: jest.fn(),
+        warn: warnMock,
+        error: jest.fn(),
+      };
+
+      process.env['GITPOD_LOG'] = 'not a log level';
+      const client = new Gitpod({ logger: logger, logLevel: 'debug', bearerToken: 'My Bearer Token' });
+      expect(client.logLevel).toBe('debug');
+      expect(warnMock).not.toHaveBeenCalled();
+    });
   });
 
   describe('defaultQuery', () => {