Skip to content

Commit 683b9fd

Browse files
zonia3000zonia3000
authored
Merge pull request #526 from fractal-analytics-platform/minor-improvements
Error formatting improvements; Added dropdown for Python version in tasks
2 parents c57647d + af194c9 commit 683b9fd

File tree

25 files changed

+245
-57
lines changed

25 files changed

+245
-57
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
# Unreleased
44

5+
* Added dropdown for Python version in tasks (\#526);
6+
* Improved formatting of errors in standard error alert component (\#526);
57
* Used `postgres-psycopg` adapter in CI (\#525);
68
* Used `FRACTAL_BACKEND_RUNNER=local_experimental` in CI for v2 tests (\#525);
79

‎__tests__/StandardErrorAlert.test.js‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
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 string detail', 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 object detail and __root__ loc', async () => {
25+
const result = render(StandardErrorAlert, {
26+
error: new AlertError(
27+
{
28+
detail: [
29+
{
30+
loc: ['body', '__root__'],
31+
msg: 'error message',
32+
type: 'value_error'
33+
}
34+
]
35+
},
36+
422
37+
)
38+
});
39+
expect(result.container.textContent).toContain('error message');
40+
expect(result.container.textContent).not.toContain('detail');
41+
expect(result.container.textContent).not.toContain('There has been an error');
42+
});
43+
44+
it('AlertError with generic object message', async () => {
45+
const result = render(StandardErrorAlert, {
46+
error: new AlertError({ foo: 'bar' })
47+
});
48+
expect(result.container.textContent).toMatch(/{.*"foo".*:.*"bar".*}/s);
49+
expect(result.container.textContent).toContain('There has been an error');
50+
});
51+
52+
it('AlertError with generic object message', async () => {
53+
const result = render(StandardErrorAlert, {
54+
error: new AlertError({ foo: 'bar' })
55+
});
56+
expect(result.container.textContent).toMatch(/{.*"foo".*:.*"bar".*}/s);
57+
expect(result.container.textContent).toContain('There has been an error');
58+
});
59+
60+
it('Generic error with string message', async () => {
61+
const result = render(StandardErrorAlert, {
62+
error: new Error('error message')
63+
});
64+
expect(result.container.textContent).toContain('error message');
65+
expect(result.container.textContent).not.toContain('There has been an error');
66+
});
67+
68+
it('Generic object message with detail', async () => {
69+
const result = render(StandardErrorAlert, {
70+
error: { detail: 'error message' }
71+
});
72+
expect(result.container.textContent).toContain('error message');
73+
expect(result.container.textContent).not.toContain('detail');
74+
expect(result.container.textContent).not.toContain('There has been an error');
75+
});
76+
77+
it('Generic object message without detail', async () => {
78+
const result = render(StandardErrorAlert, {
79+
error: { foo: 'bar' }
80+
});
81+
expect(result.container.textContent).toMatch(/{.*"foo".*:.*"bar".*}/s);
82+
expect(result.container.textContent).toContain('There has been an error');
83+
});
84+
});

‎src/lib/common/errors.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,9 @@ function getSimpleValidationMessage(reason, statusCode) {
7373
if (!isValueError(err)) {
7474
return null;
7575
}
76+
const loc = err.loc.length > 1 && err.loc[0] === 'body' ? err.loc.slice(1) : err.loc;
7677
return {
77-
loc: err.loc.length > 1 && err.loc[0] === 'body' ? err.loc.slice(1) : err.loc,
78+
loc: loc.length === 1 && loc[0] === '__root__' ? [] : loc,
7879
msg: err.msg
7980
};
8081
}

‎src/lib/components/common/StandardErrorAlert.svelte‎

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,59 @@
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+
const simpleMessage = error.getSimpleValidationMessage();
16+
if (simpleMessage) {
17+
errorString = simpleMessage;
18+
formatAsPre = false;
19+
} else if (typeof error.reason === 'string') {
20+
errorString = error.reason;
21+
formatAsPre = false;
22+
} else if ('detail' in error.reason && typeof error.reason.detail === 'string') {
23+
errorString = error.reason.detail;
24+
formatAsPre = false;
25+
} else {
26+
errorString = JSON.stringify(error.reason, undefined, 2);
27+
formatAsPre = true;
28+
}
1229
} else if (error instanceof Error) {
13-
return error.message;
30+
errorString = error.message;
31+
formatAsPre = false;
32+
} else if (typeof error === 'object' && 'detail' in error && typeof error.detail === 'string') {
33+
errorString = error.detail;
34+
formatAsPre = false;
35+
} else {
36+
errorString = JSON.stringify(error, undefined, 2);
37+
formatAsPre = true;
1438
}
15-
return error;
1639
}
1740
1841
export function hide() {
19-
errorString = undefined
42+
errorString = '';
2043
}
2144
</script>
2245

2346
{#if errorString}
2447
<div class="alert alert-danger alert-dismissible" role="alert">
25-
<pre>There has been an error, reason:</pre>
26-
<pre>{errorString}</pre>
48+
{#if formatAsPre}
49+
<p>There has been an error, reason:</p>
50+
<pre>{errorString}</pre>
51+
{:else}
52+
{#each errorString.split('\n') as line, index}
53+
{#if index > 0}
54+
<br />
55+
{/if}
56+
{line}
57+
{/each}
58+
{/if}
2759
<button class="btn-close" data-bs-dismiss="alert" on:click={hide} />
2860
</div>
2961
{/if}

‎src/lib/components/v2/admin/UserEditor.svelte‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { page } from '$app/stores';
44
import { nullifyEmptyStrings, removeNullValues } from '$lib/common/component_utilities';
55
import {
6+
AlertError,
67
displayStandardErrorAlert,
78
getValidationMessagesMap,
89
validateErrorMapKeys
@@ -75,7 +76,10 @@
7576
if (errorsMap && validateErrorMapKeys(errorsMap, handledErrorKeys)) {
7677
validationErrors = errorsMap;
7778
} else {
78-
genericErrorAlert = displayStandardErrorAlert(result, 'genericError');
79+
genericErrorAlert = displayStandardErrorAlert(
80+
new AlertError(result, response.status),
81+
'genericError'
82+
);
7983
}
8084
return;
8185
}

‎src/lib/components/v2/jobs/JobInfoModal.svelte‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<script>
22
import StatusBadge from '$lib/components/jobs/StatusBadge.svelte';
3-
import { displayStandardErrorAlert } from '$lib/common/errors';
3+
import { AlertError, displayStandardErrorAlert } from '$lib/common/errors';
44
import { page } from '$app/stores';
55
import Modal from '../../common/Modal.svelte';
66
@@ -37,7 +37,10 @@
3737
if (response.ok) {
3838
job = result;
3939
} else {
40-
errorAlert = displayStandardErrorAlert(result, 'workflowJobError');
40+
errorAlert = displayStandardErrorAlert(
41+
new AlertError(result, response.status),
42+
'workflowJobError'
43+
);
4144
}
4245
}
4346
</script>

‎src/lib/components/v2/jobs/JobsList.svelte‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import JobInfoModal from '$lib/components/v2/jobs/JobInfoModal.svelte';
77
import JobLogsModal from '$lib/components/v2/jobs/JobLogsModal.svelte';
88
import Th from '$lib/components/common/filterable/Th.svelte';
9-
import { displayStandardErrorAlert } from '$lib/common/errors';
9+
import { AlertError, displayStandardErrorAlert } from '$lib/common/errors';
1010
import { onDestroy, onMount } from 'svelte';
1111
import { removeDuplicatedItems } from '$lib/common/component_utilities';
1212
import StandardDismissableAlert from '../../common/StandardDismissableAlert.svelte';
@@ -63,11 +63,7 @@
6363
$: tableHandler.filter(statusFilter, 'status', check.isEqualTo);
6464
$: tableHandler.filter(projectFilter, (row) => row.project_dump.id.toString(), check.isEqualTo);
6565
$: tableHandler.filter(workflowFilter, (row) => row.workflow_dump.id.toString(), check.isEqualTo);
66-
$: tableHandler.filter(
67-
datasetFilter,
68-
(row) => row.dataset_dump.id.toString(),
69-
check.isEqualTo
70-
);
66+
$: tableHandler.filter(datasetFilter, (row) => row.dataset_dump.id.toString(), check.isEqualTo);
7167
7268
$: rebuildSlimSelectOptions($rows);
7369
@@ -123,8 +119,11 @@
123119
jobCancelledMessage = 'Job cancellation request received. The job will stop in a few seconds';
124120
} else {
125121
console.error('Error stopping job');
126-
const errorResponse = await response.json();
127-
errorAlert = displayStandardErrorAlert(errorResponse, 'jobUpdatesError');
122+
const result = await response.json();
123+
errorAlert = displayStandardErrorAlert(
124+
new AlertError(result, response.status),
125+
'jobUpdatesError'
126+
);
128127
}
129128
}
130129

‎src/lib/components/v2/projects/ProjectInfoModal.svelte‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<script>
2-
import { displayStandardErrorAlert } from '$lib/common/errors';
2+
import { AlertError, displayStandardErrorAlert } from '$lib/common/errors';
33
44
// ProjectInfoModal component
55
import { projectInfoModalV2 } from '$lib/stores/projectStores';
@@ -30,14 +30,14 @@
3030
method: 'GET',
3131
credentials: 'include'
3232
});
33+
const result = await response.json();
3334
if (response.ok) {
3435
/** @type {import('$lib/types-v2.js').DatasetV2[]} */
35-
const result = await response.json();
3636
result.sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0));
3737
datasets = result;
3838
} else {
3939
datasetErrorAlert = displayStandardErrorAlert(
40-
await response.json(),
40+
new AlertError(result, response.status),
4141
'errorAlert-projectInfoModal'
4242
);
4343
}

‎src/lib/components/v2/tasks/AddSingleTask.svelte‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import Ajv from 'ajv';
33
import { replaceEmptyStrings } from '$lib/common/component_utilities';
44
import {
5+
AlertError,
56
displayStandardErrorAlert,
67
getValidationMessagesMap,
78
validateErrorMapKeys
@@ -147,7 +148,10 @@
147148
if (errorsMap && validateErrorMapKeys(errorsMap, handledErrorKeys)) {
148149
validationErrors = errorsMap;
149150
} else {
150-
errorAlert = displayStandardErrorAlert(result, 'errorAlert-createTask');
151+
errorAlert = displayStandardErrorAlert(
152+
new AlertError(result, response.status),
153+
'errorAlert-createTask'
154+
);
151155
}
152156
}
153157
}

‎src/lib/components/v2/tasks/CustomEnvTask.svelte‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import manifestSchema from './manifest_v2.json';
55
import { replaceEmptyStrings } from '$lib/common/component_utilities';
66
import {
7+
AlertError,
78
displayStandardErrorAlert,
89
getValidationMessagesMap,
910
validateErrorMapKeys
@@ -124,7 +125,10 @@
124125
if (errorsMap && validateErrorMapKeys(errorsMap, handledErrorKeys)) {
125126
validationErrors = errorsMap;
126127
} else {
127-
errorAlert = displayStandardErrorAlert(result, 'errorAlert-customEnvTask');
128+
errorAlert = displayStandardErrorAlert(
129+
new AlertError(result, response.status),
130+
'errorAlert-customEnvTask'
131+
);
128132
}
129133
}
130134
} finally {

0 commit comments

Comments
 (0)