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.

Sign up for GitHub

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

improve shell error handling #330

Merged
merged 2 commits into from
Nov 30, 2023
Merged

improve shell error handling #330

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
18109cf
improve shell error handling
fauna-chase Nov 28, 2023
9f3d12b
pr fb
fauna-chase Nov 28, 2023
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
30 changes: 13 additions & 17 deletions src/commands/eval.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,25 +171,21 @@ class EvalCommand extends FaunaCommand {
}

async performV10Query(client, fqlQuery, outputFile, flags) {
try {
let format;
if (flags.format === "shell") {
format = "decorated";
} else if (flags.format === "json-tagged") {
format = "tagged";
} else {
format = "simple";
}
let format;
if (flags.format === "shell") {
format = "decorated";
} else if (flags.format === "json-tagged") {
format = "tagged";
} else {
format = "simple";
}

const res = await client.query(fqlQuery, {
format,
typecheck: flags.typecheck,
});
const res = await client.query(fqlQuery, {
format,
typecheck: flags.typecheck,
});

return await this.writeFormattedOutputV10(outputFile, res, flags.format);
} catch (error) {
this.error(`${error.code}\n\n${error.queryInfo.summary}`);
}
return this.writeFormattedOutputV10(outputFile, res, flags.format);
}

async performV4Query(client, fqlQuery, outputFile, flags) {
Expand Down
19 changes: 12 additions & 7 deletions src/commands/shell.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,22 @@ class ShellCommand extends EvalCommand {
if (cmd.trim() === "") return cb();

if (this.flags.version === "10") {
const res = await this.performV10Query(
this.connection.client,
cmd,
null,
{
let res;
try {
res = await this.performV10Query(this.connection.client, cmd, null, {
format: "shell",
version: "10",
typecheck: this.flags.typecheck,
});
} catch (err) {
let errString = "";
if (err.code) {
errString = errString.concat(`${err.code}\n`);
}
);

errString = errString.concat(err.message);
console.error(errString);
return cb(null);
}
console.log(res);

return cb(null);
Expand Down
10 changes: 6 additions & 4 deletions src/lib/environment-factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { confirm, input } from "@inquirer/prompts";
import { Command, ux } from "@oclif/core";
import { Endpoint, ShellConfig } from "./config";
import FaunaClient, { QuerySuccess } from "./fauna-client";
import FaunaClient, { QueryFailure, QuerySuccess } from "./fauna-client";
import { searchSelect } from "./search-select";

export interface AddEnvironmentParams {
Expand Down Expand Up @@ -112,7 +112,9 @@ export class EnvironmentFactory {
database: databaseName,
};
this.config.saveProjectConfig();
console.log(`Saved environment ${name} to ${this.config.projectConfigFile()}`);
console.log(
`Saved environment ${name} to ${this.config.projectConfigFile()}`
);
}

promptDatabasePath = async (endpoint: Endpoint): Promise<string> => {
Expand All @@ -124,7 +126,7 @@ export class EnvironmentFactory {

const res = await client.query("0");
if (res.status !== 200) {
this.cmd.error(`${res.body.error.code}`);
this.cmd.error(`${(res as QueryFailure).body.error.code}`);
}

const databasePaths = await this.getDatabasePaths(client);
Expand Down Expand Up @@ -199,7 +201,7 @@ export class EnvironmentFactory {
}
);
if (databases.status !== 200) {
this.cmd.error(`Error: ${databases.body.error.code}`);
this.cmd.error(`Error: ${(databases as QueryFailure).body.error.code}`);
}

const dbs = (databases as QuerySuccess<any>).body.data;
Expand Down
107 changes: 38 additions & 69 deletions src/lib/fauna-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,18 @@
import { connect, constants } from "http2";

// Copied from the fauna-js driver:
// https://github.com/fauna/fauna-js/blob/main/src/http-client/node-http2-client.ts
import fetch from "node-fetch";

export type QueryResponse<T> = QuerySuccess<T> | QueryFailure;

export type QueryInfo = {
headers: any;
body: {
summary: string;
};
};

export type QuerySuccess<T> = QueryInfo & {
export type QuerySuccess<T> = {
status: 200;
body: {
data: T;
};
};

export type QueryFailure = QueryInfo & {
status: 400;
export type QueryFailure = {
status: number;
body: {
summary?: string;
error: {
code: string;
message?: string;
Expand All @@ -30,16 +21,12 @@ export type QueryFailure = QueryInfo & {
};

export default class FaunaClient {
session: any;
endpoint: string;
secret: string;
timeout?: number;

constructor(opts: { endpoint: string; secret: string; timeout?: number }) {
this.session = connect(opts.endpoint, {
peerMaxConcurrentStreams: 50,
})
.once("error", () => this.close())
.once("goaway", () => this.close());
this.endpoint = opts.endpoint;
this.secret = opts.secret;
this.timeout = opts.timeout;
}
Expand All @@ -57,57 +44,39 @@ export default class FaunaClient {
typecheck: opts?.typecheck ?? undefined,
secret: opts?.secret ?? this.secret,
};
return new Promise((resolvePromise, rejectPromise) => {
let req: any;
const onResponse = (http2ResponseHeaders: any) => {
const status = http2ResponseHeaders[constants.HTTP2_HEADER_STATUS];
let responseData = "";
const url = new URL(this.endpoint);
url.pathname = "/query/1";
const res = await fetch(url, {
method: "POST",
headers: {
authorization: `Bearer ${secret ?? this.secret}`,
"x-fauna-source": "Fauna Shell",
...(typecheck !== undefined && { "x-typecheck": typecheck.toString() }),
...(format !== undefined && { "x-format": format }),
Comment on lines +53 to +56
Copy link
Contributor

Choose a reason for hiding this comment

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

the x-query-timeout-ms header should be sent here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call, looks like we also weren't correctly passing the timeout to the client so resolving that as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh cool, nice

},
body: JSON.stringify({ query }),
});

req.on("data", (chunk: any) => {
responseData += chunk;
});
const json = await res.json();

req.on("end", () => {
resolvePromise({
status,
body: JSON.parse(responseData),
headers: http2ResponseHeaders,
});
});
if (res.status === 200 || res.status === 201) {
return {
status: 200,
body: {
data: json.data as T,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

this should just be

Suggested change
body: {
data: json.data as T,
},
body: json,

so that the summary field gets copied over. for example, try this: fauna eval "log('hi')"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

o I see, thats why the summary field was shared across error and success types

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah. you can also get log messages back in the summary for runtime errors too:

log("hi"); 1/0

// returns

divide_by_zero: Attempted integer division by zero.

error: Attempted integer division by zero.
at *query*:1:14
  |
1 | log('hi'); 1/0
  |              ^
  |

info at *query*:1: hi

};

try {
const httpRequestHeaders = {
Authorization: `Bearer ${secret}`,
"x-format": format,
"X-Fauna-Source": "Fauna Shell",
[constants.HTTP2_HEADER_PATH]: "/query/1",
[constants.HTTP2_HEADER_METHOD]: "POST",
...((typecheck && { "x-typecheck": typecheck }) ?? {}),
...((this.timeout && { "x-query-timeout-ms": this.timeout }) ?? {}),
};

req = this.session
.request(httpRequestHeaders)
.setEncoding("utf8")
.on("error", (error: any) => rejectPromise(error))
.on("response", onResponse);

req.write(JSON.stringify({ query }), "utf8");

// req.setTimeout must be called before req.end()
req.setTimeout((this.timeout ?? 0) + 5000, () => {
req.destroy(new Error(`Client timeout`));
});

req.end();
} catch (error) {
rejectPromise(error);
}
});
}

async close() {
this.session.close();
} else {
return {
status: res.status,
body: {
summary: json.summary,
error: {
code: json.error?.code,
message: json.error?.message,
},
},
};
}
}
}