EdgeDB Cloud on Deno silently requires `--allow-env`

[mmastrac]

Describe the bug

When connecting to EdgeDB cloud, the required EDGEDB_* environment variables need to be read to retrieve the required instance and secret key. The Deno adapter silently avoids reading these variables when --allow-env is not present, causing the EdgeDB client to fall back to a local connection in most cases.

This may appear as if the client is not successfully connecting to the EdgeDB cloud.

Reproduction

export EDGEDB_INSTANCE=<name>/<db>
export EDGEDB_SECRET_KEY=<key>

# Note `deno repl` to exclude all permissions and prompt
$ cd /tmp/
$ deno repl
Deno 1.44.1
exit using ctrl+d, ctrl+c, or close()
> import * as edgedb from "https://deno.land/x/edgedb/mod.ts";
✅ Granted net access to "deno.land".
undefined
> const client = edgedb.createClient();
undefined
> await client.querySingle('select 1 + 1');
✅ Granted read access to <CWD>.
✅ Granted read access to "/Users/matt/Documents/github".
✅ Granted read access to "/Users/matt/Documents".
✅ Granted read access to "/Users/matt".
✅ Granted read access to "/Users".
✅ Granted read access to "/".
Uncaught ClientConnectionError: no 'edgedb.toml' found and no connection options specified either via arguments to `createClient()` API or via environment variables EDGEDB_HOST, EDGEDB_INSTANCE, EDGEDB_DSN, EDGEDB_CREDENTIALS or EDGEDB_CREDENTIALS_FILE

Note that --allow-env works as you'd expect:

$ cd /tmp/
$ deno repl --allow-env
Deno 1.44.1
exit using ctrl+d, ctrl+c, or close()
> import * as edgedb from "https://deno.land/x/edgedb/mod.ts";
✅ Granted net access to "deno.land".
undefined
> const client = edgedb.createClient();
undefined
> await client.querySingle('select 1 + 1');
✅ Granted net access to "<db>:<port>".
2

The problem appears to be that the client silently returns undefined here -- the same result as if the env var doesn't exist.

export function getEnv(envName: string, required = false): string | undefined {
  if (!required) {
    const state = Deno.permissions.querySync({
      name: "env",
      variable: envName,
    }).state;
    if (state !== "granted") {
      return undefined;
    }
  }
  return Deno.env.get(envName);
}

Expected behavior

As this can cause end-user confusion, it's best if the client behaves in one of three ways:

1. It should attempt to read environment variables unconditionally. This will have the tradeoff of being very noisy when permissions are set to "prompt", but will encourage the user to set the permissions appropriately. The required permissions string may be fairly long, however.
2. If the environment permission is not set, it could console.warn with the variables that were skipped and encourage the user to specify the connection string explicitly and/or configure the --allow-env permission.
3. If the environment permission is not set, it could throw an exception and require the client to specify the connection strings (ideally with a hint in the thrown exception).

Versions (please complete the following information):

Deno: approx >1.33 (versions with stable ALPN support) edgedb-js: current (< 1.5.7)

[scotttrinh]

I think I'm leaning toward 1: our client connections really do want to try to read from the environment, so even if you don't plan on using environment variables to configure the client (which is against our best practices), maybe we should still attempt looking there.

At the very least we should update the Deno readme to suggest allow-env and allow-read permissions in stronger language and call out that our connection algorithm really wants this.