Skip to content

Commit

Permalink
fix: better error message when providing a command builder to an expr (
Browse files Browse the repository at this point in the history
  • Loading branch information
dsherret committed Feb 5, 2024
1 parent 27ab9cb commit 67728a2
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 14 deletions.
14 changes: 13 additions & 1 deletion mod.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2108,12 +2108,24 @@ Deno.test("should support empty quoted string", async () => {
assertEquals(output, " test ");
});

Deno.test("esnure deprecated PathRef export still works", () => {
Deno.test("ensure deprecated PathRef export still works", () => {
const path = new PathRef("hello");
assert(path instanceof Path);
assert(path instanceof PathRef);
});

Deno.test("nice error message when not awaiting a CommandBuilder", async () => {
await assertRejects(
async () => {
const cmd = $`echo 1`;
return await $`echo ${cmd}`;
},
Error,
"Providing a command builder is not yet supported (https://github.com/dsherret/dax/issues/239). " +
"Await the command builder's text before using it in an expression (ex. await $`cmd`.text()).",
);
});

function ensurePromiseNotResolved(promise: Promise<unknown>) {
return new Promise<void>((resolve, reject) => {
promise.then(() => reject(new Error("Promise was resolved")));
Expand Down
10 changes: 5 additions & 5 deletions mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {
CommandBuilder,
escapeArg,
getRegisteredCommandNamesSymbol,
setCommandTextAndFdsSymbol,
setCommandTextStateSymbol,
template,
templateRaw,
} from "./src/command.ts";
Expand Down Expand Up @@ -632,8 +632,8 @@ function build$FromState<TExtras extends ExtrasObject = {}>(state: $State<TExtra
};
const result = Object.assign(
(strings: TemplateStringsArray, ...exprs: any[]) => {
const { text, streams } = template(strings, exprs);
return state.commandBuilder.getValue()[setCommandTextAndFdsSymbol](text, streams);
const textState = template(strings, exprs);
return state.commandBuilder.getValue()[setCommandTextStateSymbol](textState);
},
helperObject,
logDepthObj,
Expand Down Expand Up @@ -764,8 +764,8 @@ function build$FromState<TExtras extends ExtrasObject = {}>(state: $State<TExtra
return state.requestBuilder.url(url);
},
raw(strings: TemplateStringsArray, ...exprs: any[]) {
const { text, streams } = templateRaw(strings, exprs);
return state.commandBuilder.getValue()[setCommandTextAndFdsSymbol](text, streams);
const textState = templateRaw(strings, exprs);
return state.commandBuilder.getValue()[setCommandTextStateSymbol](textState);
},
withRetries<TReturn>(opts: RetryOptions<TReturn>): Promise<TReturn> {
return withRetries(result, state.errorLogger.getValue(), opts);
Expand Down
18 changes: 10 additions & 8 deletions src/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ const builtInCommands = {
/** @internal */
export const getRegisteredCommandNamesSymbol: unique symbol = Symbol();
/** @internal */
export const setCommandTextAndFdsSymbol: unique symbol = Symbol();
export const setCommandTextStateSymbol: unique symbol = Symbol();

/**
* Underlying builder API for executing commands.
Expand Down Expand Up @@ -583,12 +583,9 @@ export class CommandBuilder implements PromiseLike<CommandResult> {
}

/** @internal */
[setCommandTextAndFdsSymbol](text: string, fds: StreamFds | undefined) {
[setCommandTextStateSymbol](textState: CommandBuilderStateCommand) {
return this.#newWithState((state) => {
state.command = {
text,
fds,
};
state.command = textState;
});
}
}
Expand Down Expand Up @@ -1240,7 +1237,7 @@ function templateInner(
strings: TemplateStringsArray,
exprs: any[],
escape: ((arg: string) => string) | undefined,
) {
): CommandBuilderStateCommand {
let nextStreamFd = 3;
let text = "";
let streams: StreamFds | undefined;
Expand Down Expand Up @@ -1380,7 +1377,7 @@ function templateInner(
}
return {
text,
streams,
fds: streams,
};

function handleReadableStream(createStream: () => ReadableStream) {
Expand Down Expand Up @@ -1440,6 +1437,11 @@ function templateLiteralExprToString(expr: any, escape: ((arg: string) => string
} else if (expr instanceof CommandResult) {
// remove last newline
result = expr.stdout.replace(/\r?\n$/, "");
} else if (expr instanceof CommandBuilder) {
throw new Error(
"Providing a command builder is not yet supported (https://github.com/dsherret/dax/issues/239). " +
"Await the command builder's text before using it in an expression (ex. await $`cmd`.text()).",
);
} else if (typeof expr === "object" && expr.toString === Object.prototype.toString) {
throw new Error("Provided object does not override `toString()`.");
} else {
Expand Down

0 comments on commit 67728a2

Please sign in to comment.