chore: improve error handing for fetch function #267
Conversation
Signed-off-by: Mirko Mollik <[email protected]>
cre8
requested review from
lukasjhan,
aceshim,
zustkeeper and
berendsliedrecht
as code owners
Signed-off-by: Mirko Mollik <[email protected]>
| } | ||
| await this.validateIntegrity(response.clone(), url, integrity); | ||
| return response.json() as Promise<T>; | ||
| } catch (error) { | ||
| if (error instanceof DOMException && error.name === 'AbortError') { |
There was a problem hiding this comment.
In node.js or react native environments, is this still a DOMException?
There was a problem hiding this comment.
Hmm.. It could be a problem in other platform. I missed exception handling part.
@cre8 I think we should find other way to handle it
There was a problem hiding this comment.
@berendsliedrecht @lukasjhan I think we should write a test and find it out. I added this instance of check, otherwise the linter would fail because of the missing type.
I will open a new PR to add tests and to correct it
There was a problem hiding this comment.
@berendsliedrecht I ran the test in node pnpm run test:node and it threw me the DomException. I am not sure how it will work with react native since we have no chance here to test it...
One solution would be to replace fetch via a http library like axios. But this will add another external lib
Improve the fetch function based on the input from #258 + adding with timeout case and allowing to adjust the timeout value