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

Surface Export Errors #3115

Merged
merged 6 commits into from
Jun 26, 2024
Merged
Changes from all 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
9 changes: 8 additions & 1 deletion apps/zui/src/core/invoke.ts
Original file line number Diff line number Diff line change
@@ -4,5 +4,12 @@ export function invoke<K extends OperationName>(
name: K,
...args: Parameters<Operations[K]>
): Promise<Awaited<ReturnType<Operations[K]>>> {
return global.zui.invoke(name, ...args)
return global.zui.invoke(name, ...args).catch((error) => {
throw new Error(
error
.toString()
// Here we are stripping the error prefix that electron tacks on.
.replace(`Error: Error invoking remote method '${name}': `, "")
)
})
}
4 changes: 4 additions & 0 deletions apps/zui/src/css/global/_forms.scss
Original file line number Diff line number Diff line change
@@ -37,6 +37,10 @@ select {
background-repeat: no-repeat;
background-size: 1.2em 1.2em;
background-position: calc(100% - 10px) center;
field-sizing: content;
inline-size: auto;
max-inline-size: 100%;
padding-inline-end: 3em;
}

label {
2 changes: 1 addition & 1 deletion apps/zui/src/domain/results/handlers/export.ts
Original file line number Diff line number Diff line change
@@ -44,7 +44,7 @@ export const exportToFile = createHandler(
.promise(promise, {
loading: "Exporting...",
success: "Export Completed: " + filePath,
error: "Error Exporting",
error: (e) => e.message,
})
.catch((e) => {
console.error(e)
5 changes: 5 additions & 0 deletions apps/zui/src/domain/results/operations.ts
Original file line number Diff line number Diff line change
@@ -17,11 +17,16 @@ export const exportToFile = createOperation(
controlMessages: false,
timeout: Infinity,
})

try {
await pipe(
res.body as unknown as NodeJS.ReadableStream,
fs.createWriteStream(outPath)
)
const status = await lake.client.queryStatus(res.requestId)
if (status.error) {
throw new Error(status.error)
}
} catch (e) {
fs.unlink(outPath, () => {})
throw e
10 changes: 10 additions & 0 deletions packages/zed-js/src/client/base-client.ts
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@ import { ResultStream } from '../query/result-stream';
import { createError } from '../util/error';
import * as Types from './types';
import { accept, defaults, parseContent, toJS, wrapAbort } from './utils';
import { decode } from '..';

export abstract class BaseClient {
public abstract fetch: Types.IsoFetch;
@@ -50,6 +51,15 @@ export abstract class BaseClient {
return new ResultStream(result, abortCtl);
}

async queryStatus(id: string) {
const response = await this.send({
method: 'GET',
path: `/query/status/${id}`,
contentType: 'application/json',
});
return decode(await response.json()).toJS();
}

async describeQuery(
query: string,
options: { signal?: AbortSignal; timeout?: number; pool?: string } = {}
4 changes: 4 additions & 0 deletions packages/zed-js/src/query/result-stream.ts
Original file line number Diff line number Diff line change
@@ -17,6 +17,10 @@ export class ResultStream extends EventEmitter {
super();
}

get requestId() {
return this.resp.headers.get('x-request-id');
}

get body() {
return this.resp.body;
}
6 changes: 6 additions & 0 deletions packages/zui-player/helpers/test-app.ts
Original file line number Diff line number Diff line change
@@ -260,6 +260,12 @@ export default class TestApp {
fullPage: true,
});
}

mockSaveDialog(result: { canceled: boolean; filePath: string | null }) {
this.zui.evaluate(async ({ dialog }, result) => {
dialog.showSaveDialog = () => Promise.resolve(result);
}, result);
}
}

const getAppInfo = () => {
18 changes: 14 additions & 4 deletions packages/zui-player/tests/export.spec.ts
Original file line number Diff line number Diff line change
@@ -40,10 +40,7 @@ test.describe('Export tests', () => {
formats.forEach(({ label, expectedSize }) => {
test(`Exporting in ${label} format succeeds`, async () => {
const file = path.join(tempDir, `results.${label}`);
app.zui.evaluate(async ({ dialog }, filePath) => {
dialog.showSaveDialog = () =>
Promise.resolve({ canceled: false, filePath });
}, file);
app.mockSaveDialog({ canceled: false, filePath: file });
await app.click('button', 'Export Results');
await app.attached('dialog');
const dialog = app.mainWin.getByRole('dialog');
@@ -84,4 +81,17 @@ test.describe('Export tests', () => {
await app.click('button', 'Query Pool');
await app.attached(/5 Total Rows/);
});

test('Export with error', async () => {
const filePath = path.join(tempDir, `error.csv`);
app.mockSaveDialog({ canceled: false, filePath });
await app.query('yield uid');
await app.click('button', 'Export Results');
await app.attached('dialog');
await app.locate('radio', 'File').check();
await app.select('Format', 'CSV');
await app.click('button', 'Export to File ↩');
await app.attached(/Error: CSV output encountered non-record value/);
await expect(fsExtra.stat(filePath)).rejects.toThrowError('no such file');
});
});