-
Notifications
You must be signed in to change notification settings - Fork 3
Improve behavior on errors when importing a dataset #74
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
Changes from all commits
2453814
7982b5d
69b2d90
64afa04
be5b033
20e8fc9
cc81acb
9e4cb11
b0c541e
85662ec
5c244f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| /* eslint-disable @typescript-eslint/restrict-template-expressions */ | ||
| import UIkit from 'uikit'; | ||
| import { appendIssueToTitle } from '../components/dialogs/utils'; | ||
| import { | ||
|
|
@@ -43,11 +44,23 @@ const ENDPOINT = process.env.EPIDATA_ENDPOINT_URL; | |
|
|
||
| export const fetchOptions: RequestInit = process.env.NODE_ENV === 'development' ? { cache: 'force-cache' } : {}; | ||
|
|
||
| function processResponse<T>(response: Response): Promise<T> { | ||
| if (response.ok) { | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| return response.json(); | ||
| } | ||
| return response.text().then((text) => { | ||
| throw new Error(`[${response.status}] ${text}`); | ||
| }); | ||
| } | ||
|
|
||
| export function fetchImpl<T>(url: URL): Promise<T> { | ||
| const urlGetS = url.toString(); | ||
| if (urlGetS.length < 4096) { | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| return fetch(url.toString(), fetchOptions).then((d) => d.json()); | ||
| return fetch(url.toString(), fetchOptions).then((d) => { | ||
| return processResponse(d); | ||
| }); | ||
| } | ||
| const params = new URLSearchParams(url.searchParams); | ||
| url.searchParams.forEach((d) => url.searchParams.delete(d)); | ||
|
|
@@ -56,7 +69,9 @@ export function fetchImpl<T>(url: URL): Promise<T> { | |
| ...fetchOptions, | ||
| method: 'POST', | ||
| body: params, | ||
| }).then((d) => d.json()); | ||
| }).then((d) => { | ||
| return processResponse(d); | ||
| }); | ||
| } | ||
|
|
||
| // generic epidata loader | ||
|
|
@@ -147,26 +162,50 @@ export function loadDataSet( | |
| url.searchParams.set('format', 'json'); | ||
| return fetchImpl<Record<string, unknown>[]>(url) | ||
| .then((res) => { | ||
| const data = loadEpidata(title, res, columns, columnRenamings, { _endpoint: endpoint, ...params }); | ||
| if (data.datasets.length == 0) { | ||
| try { | ||
| const data = loadEpidata(title, res, columns, columnRenamings, { _endpoint: endpoint, ...params }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we put the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this is already the case? The try block is around three lines: And the last two can't really error out.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thats what im saying -- the |
||
| if (data.datasets.length == 0) { | ||
| return UIkit.modal | ||
| .alert( | ||
| ` | ||
| <div class="uk-alert uk-alert-error"> | ||
| <a href="${url.href}">API Link</a> returned no data, which suggests that the API has no available information for the selected location. | ||
| </div>`, | ||
| ) | ||
| .then(() => null); | ||
| } | ||
| return data; | ||
| } catch (error) { | ||
| console.warn('failed loading data', error); | ||
| // EpiData API error - JSON with "message" property | ||
| if ('message' in res) { | ||
| return UIkit.modal | ||
| .alert( | ||
| ` | ||
| <div class="uk-alert uk-alert-error"> | ||
| [f01] Failed to fetch API data from <a href="${url.href}">API Link</a>:<br/><i>${res['message']}</i> | ||
| </div>`, | ||
| ) | ||
| .then(() => null); | ||
| } | ||
| // Fallback for generic error | ||
| return UIkit.modal | ||
| .alert( | ||
| ` | ||
| <div class="uk-alert uk-alert-error"> | ||
| <a href="${url.href}">API Link</a> returned no data. | ||
| [f02] Failed to fetch API data from <a href="${url.href}">API Link</a>:<br/><i>${error}</i> | ||
| </div>`, | ||
| ) | ||
| .then(() => null); | ||
| } | ||
| return data; | ||
| }) | ||
| .catch((error) => { | ||
| console.warn('failed fetching data', error); | ||
| return UIkit.modal | ||
| .alert( | ||
| ` | ||
| <div class="uk-alert uk-alert-error"> | ||
| Failed to fetch API data from <a href="${url.href}">API Link</a>. | ||
| [f03] Failed to fetch API data from <a href="${url.href}">API Link</a>:<br/><i>${error}</i> | ||
| </div>`, | ||
| ) | ||
| .then(() => null); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would this work (or something similar)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
explored in #86