Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix Connected to undefined message #284

Merged
merged 4 commits into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions src/commands/shell.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const FaunaCommand = require("../lib/fauna-command.js").default;
const { runQueries, stringifyEndpoint } = require("../lib/misc.js");
const { runQueries } = require("../lib/misc.js");
const faunadb = require("faunadb");
const { Flags, Args } = require("@oclif/core");
const q = faunadb.query;
Expand Down Expand Up @@ -41,9 +41,6 @@ class ShellCommand extends EvalCommand {
this.log(`Starting shell for database ${db_path}`);
}

this.log(
`Connected to ${stringifyEndpoint(this.connection.connectionOptions)}`
);
this.log("Type Ctrl+D or .exit to exit the shell");

this.repl = repl.start({
Expand Down
5 changes: 5 additions & 0 deletions src/lib/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ export type ShellOpts = {
export type EndpointConfig = {
secret: string;
url: string;
/** this is currently just used for messaging purposes, the secret
* in this config will already contain the database path if needed.
*/
database?: string;
name?: string;
graphqlHost: string;
graphqlPort: number;
};
Expand Down
31 changes: 16 additions & 15 deletions src/lib/config/root-config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Config, InvalidConfigError } from ".";
import { Config, EndpointConfig, InvalidConfigError } from ".";
import fs from "fs";
const ini = require("ini");

Expand All @@ -23,13 +23,13 @@ export class RootConfig {
this.endpoints = Object.fromEntries<Endpoint>(
config
.objectsIn("endpoint")
.map(([k, v]) => [k, Endpoint.fromConfig(v)])
.map(([k, v]) => [k, Endpoint.fromConfig(k, v)])
);
} else {
this.endpoints = Object.fromEntries<Endpoint>(
config
.allObjectsWhere((k) => k !== "default")
.map(([k, v]) => [k, Endpoint.fromConfig(v)])
.map(([k, v]) => [k, Endpoint.fromConfig(k, v)])
);
}

Expand Down Expand Up @@ -101,14 +101,16 @@ export class RootConfig {
* - An optional GraphQL port. This is the port to use for GraphQL requests. It defaults to `443`.
*/
export class Endpoint {
name?: string;
macmv marked this conversation as resolved.
Show resolved Hide resolved
secret: string;
url: string;

graphqlHost: string;
graphqlPort: number;

static fromConfig(config: Config) {
static fromConfig(name: string, config: Config) {
return new Endpoint({
name: name,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style:

Suggested change
name: name,
name,

secret: config.str("secret"),
url: Endpoint.getURLFromConfig(config),

Expand All @@ -118,41 +120,40 @@ export class Endpoint {
}

constructor(opts: {
name?: string;
macmv marked this conversation as resolved.
Show resolved Hide resolved
secret: string;
url?: string;
graphqlHost?: string;
graphqlPort?: number;
}) {
this.name = opts.name;
this.secret = opts.secret;
this.url = opts.url ?? "https://db.fauna.com";

this.graphqlHost = opts.graphqlHost ?? "graphql.fauna.com";
this.graphqlPort = opts.graphqlPort ?? 443;
}

makeScopedEndpoint(
scope?: string,
role?: string
): {
secret: string;
url: string;
graphqlHost: string;
graphqlPort: number;
} {
makeScopedEndpoint(databaseScope?: string, role?: string): EndpointConfig {
let appendedRoleStr = "";
if (role) {
appendedRoleStr = `:${role}`;
} else if (scope) {
} else if (databaseScope) {
appendedRoleStr = ":admin";
}

const secret = this.secret + (scope ? `:${scope}` : "") + appendedRoleStr;
const secret =
this.secret +
(databaseScope ? `:${databaseScope}` : "") +
appendedRoleStr;

return {
secret,
name: this.name,
url: this.url,
graphqlHost: this.graphqlHost,
graphqlPort: this.graphqlPort,
database: databaseScope,
};
}

Expand Down
36 changes: 28 additions & 8 deletions src/lib/fauna-command.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Command, Flags } from "@oclif/core";
import { ShellConfig } from "./config";
import { stringifyEndpoint } from "./misc";
import { query as q, errors, Client } from "faunadb";
import { green } from "chalk";
import FaunaClient from "./fauna-client";
Expand Down Expand Up @@ -84,16 +83,35 @@ class FaunaCommand extends Command {

mapConnectionError({ err, connectionOptions }) {
if (err instanceof errors.Unauthorized) {
return this.error(
`Could not Connect to ${stringifyEndpoint(
connectionOptions
)} Unauthorized Secret`
this.error(
`Could not Connect to ${connectionOptions.url} Unauthorized Secret`
Copy link
Contributor

@macmv macmv Oct 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`Could not Connect to ${connectionOptions.url} Unauthorized Secret`
`Could not connect to ${connectionOptions.url}: unauthorized secret`

also it might be nice to print the secret there? not sure if we want to expose that though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm yeah I'm not sure that we want to do that, my bias would be to just leave it out

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good to me

);
}
return this.error(err);
this.error(err);
}

async getClient({ dbScope, role, version } = {}) {
const logConnectionMessage = (connectionOptions) => {
let connectedMessage;
if (connectionOptions.name !== undefined) {
connectedMessage = `Connected to endpoint: ${connectionOptions.name}`;
if (
connectionOptions.database !== undefined &&
connectionOptions.database !== ""
) {
connectedMessage += ` database: ${connectionOptions.database}`;
}
} else if (
connectionOptions.database !== undefined &&
connectionOptions.database !== ""
) {
connectedMessage = `Connected to database: ${connectionOptions.database}`;
}
if (connectedMessage !== undefined) {
this.log(connectedMessage);
}
};

if (version === "4" || version === undefined) {
// construct v4 client
let connectionOptions;
Expand Down Expand Up @@ -128,9 +146,10 @@ class FaunaCommand extends Command {

const hashKey = [dbScope, role].join("_");
this.clients[hashKey] = { client, connectionOptions };
logConnectionMessage(connectionOptions);
return this.clients[hashKey];
} catch (err) {
return this.mapConnectionError({ err, connectionOptions });
this.mapConnectionError({ err, connectionOptions });
}
} else {
// construct v10 client
Expand All @@ -156,9 +175,10 @@ class FaunaCommand extends Command {
client,
connectionOptions,
};
logConnectionMessage(connectionOptions);
return this.clients[hashKey];
} catch (err) {
return this.mapConnectionError({ err, connectionOptions });
this.mapConnectionError({ err, connectionOptions });
}
}
}
Expand Down
12 changes: 0 additions & 12 deletions src/lib/misc.js
Original file line number Diff line number Diff line change
Expand Up @@ -382,15 +382,3 @@ export function runQueries(expressions, client) {
return promiseSerial(wrapQueries(expressions, client));
}
}

export function stringifyEndpoint(endpoint) {
var res = "";
if (endpoint.scheme) {
res += endpoint.scheme + "://";
}
res += endpoint.domain;
if (endpoint.port) {
res += ":" + endpoint.port;
}
return res;
}
6 changes: 6 additions & 0 deletions test/commands/endpoint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,15 @@ describe("endpoint:add", () => {
"my-endpoint": {
url: "http://bar.baz",
secret: "fn3333",
name: "my-endpoint",
// These graphql bits are only saved if they differ from the
// default.
graphqlHost: "graphql.fauna.com",
graphqlPort: 443,
},
foobar: {
url: "http://foo.baz",
name: undefined,
secret: "fn1234",
graphqlHost: "graphql.fauna.com",
graphqlPort: 443,
Expand Down Expand Up @@ -106,6 +108,7 @@ describe("endpoint:add", () => {
"my-endpoint": {
url: "http://bar.baz",
secret: "fn3333",
name: "my-endpoint",
// These graphql bits are only saved if they differ from the
// default.
graphqlHost: "graphql.fauna.com",
Expand All @@ -114,6 +117,7 @@ describe("endpoint:add", () => {
foobar: {
url: "http://foo.baz",
secret: "fn1234",
name: undefined,
macmv marked this conversation as resolved.
Show resolved Hide resolved
graphqlHost: "graphql.fauna.com",
graphqlPort: 443,
},
Expand Down Expand Up @@ -180,6 +184,7 @@ describe("endpoint:remove", () => {
"my-endpoint": {
url: "http://bar.baz",
secret: "fn3333",
name: "my-endpoint",
// These graphql bits are only saved if they differ from the
// default.
graphqlHost: "graphql.fauna.com",
Expand Down Expand Up @@ -218,6 +223,7 @@ describe("endpoint:remove", () => {
"other-endpoint": {
url: "http://bar.baz",
secret: "fn3333",
name: "other-endpoint",
// These graphql bits are only saved if they differ from the
// default.
graphqlHost: "graphql.fauna.com",
Expand Down