Skip to content

Commit 946f33b

Browse files
committed
Improved error formatting
1 parent c57647d commit 946f33b

File tree

3 files changed

+97
-10
lines changed

3 files changed

+97
-10
lines changed
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { render } from '@testing-library/svelte';
3+
4+
import StandardErrorAlert from '../src/lib/components/common/StandardErrorAlert.svelte';
5+
import { AlertError } from '../src/lib/common/errors';
6+
7+
describe('StandardErrorAlert', () => {
8+
it('AlertError with string message', async () => {
9+
const result = render(StandardErrorAlert, {
10+
error: new AlertError('error message')
11+
});
12+
expect(result.container.textContent).toContain('error message');
13+
});
14+
15+
it('AlertError with detail message', async () => {
16+
const result = render(StandardErrorAlert, {
17+
error: new AlertError({ detail: 'error message' })
18+
});
19+
expect(result.container.textContent).toContain('error message');
20+
expect(result.container.textContent).not.toContain('detail');
21+
expect(result.container.textContent).not.toContain('There has been an error');
22+
});
23+
24+
it('AlertError with generic object message', async () => {
25+
const result = render(StandardErrorAlert, {
26+
error: new AlertError({ foo: 'bar' })
27+
});
28+
expect(result.container.textContent).toMatch(/{.*"foo".*:.*"bar".*}/s);
29+
expect(result.container.textContent).toContain('There has been an error');
30+
});
31+
32+
it('AlertError with generic object message', async () => {
33+
const result = render(StandardErrorAlert, {
34+
error: new AlertError({ foo: 'bar' })
35+
});
36+
expect(result.container.textContent).toMatch(/{.*"foo".*:.*"bar".*}/s);
37+
expect(result.container.textContent).toContain('There has been an error');
38+
});
39+
40+
it('Generic error with string message', async () => {
41+
const result = render(StandardErrorAlert, {
42+
error: new Error('error message')
43+
});
44+
expect(result.container.textContent).toContain('error message');
45+
expect(result.container.textContent).not.toContain('There has been an error');
46+
});
47+
48+
it('Generic object message with detail', async () => {
49+
const result = render(StandardErrorAlert, {
50+
error: { detail: 'error message' }
51+
});
52+
expect(result.container.textContent).toContain('error message');
53+
expect(result.container.textContent).not.toContain('detail');
54+
expect(result.container.textContent).not.toContain('There has been an error');
55+
});
56+
57+
it('Generic object message without detail', async () => {
58+
const result = render(StandardErrorAlert, {
59+
error: { foo: 'bar' }
60+
});
61+
expect(result.container.textContent).toMatch(/{.*"foo".*:.*"bar".*}/s);
62+
expect(result.container.textContent).toContain('There has been an error');
63+
});
64+
});

src/lib/components/common/StandardErrorAlert.svelte

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,50 @@
33
44
export let error = undefined;
55
6-
/** @type {string | undefined} */
7-
$: errorString = JSON.stringify(getErrorValue(), undefined, 2);
6+
let errorString = '';
7+
let formatAsPre = false;
88
9-
function getErrorValue() {
9+
$: if (error) {
10+
updateErrorMessage();
11+
}
12+
13+
function updateErrorMessage() {
1014
if (error instanceof AlertError) {
11-
return error.reason;
15+
if (typeof error.reason === 'string') {
16+
errorString = error.reason;
17+
formatAsPre = false;
18+
} else if ('detail' in error.reason) {
19+
errorString = /** @type {string}*/ (error.reason.detail);
20+
formatAsPre = false;
21+
} else {
22+
errorString = JSON.stringify(error.reason, undefined, 2);
23+
formatAsPre = true;
24+
}
1225
} else if (error instanceof Error) {
13-
return error.message;
26+
errorString = error.message;
27+
formatAsPre = false;
28+
} else if (typeof error === 'object' && 'detail' in error) {
29+
errorString = error.detail;
30+
formatAsPre = false;
31+
} else {
32+
errorString = JSON.stringify(error, undefined, 2);
33+
formatAsPre = true;
1434
}
15-
return error;
1635
}
1736
1837
export function hide() {
19-
errorString = undefined
38+
errorString = '';
2039
}
2140
</script>
2241

2342
{#if errorString}
2443
<div class="alert alert-danger alert-dismissible" role="alert">
25-
<pre>There has been an error, reason:</pre>
26-
<pre>{errorString}</pre>
44+
{#if formatAsPre}
45+
<p>There has been an error, reason:</p>
46+
<pre>{errorString}</pre>
47+
{:else}
48+
{errorString}
49+
{/if}
2750
<button class="btn-close" data-bs-dismiss="alert" on:click={hide} />
2851
</div>
2952
{/if}

tests/v2/images.spec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ test('Dataset images [v2]', async ({ page, project }) => {
7070
await modal.getByRole('textbox', { name: 'Zarr URL' }).fill(`${randomPath}/img1`);
7171
const saveBtn = modal.getByRole('button', { name: 'Save' });
7272
await saveBtn.click();
73-
await modal.getByText('There has been an error').waitFor();
73+
await modal.getByText(/Image with zarr_url '.*' already in Dataset/).waitFor();
7474
await modal.getByRole('button', { name: 'Cancel' }).click();
7575
await waitModalClosed(page);
7676
});

0 commit comments

Comments
 (0)