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

Fix error modal handling #177

Merged
merged 1 commit into from
Jan 25, 2024
Merged

Fix error modal handling #177

merged 1 commit into from
Jan 25, 2024

Conversation

bcspragu
Copy link
Collaborator

As discussed, this is an alternate approach to #174 for fixing #168. It handles errors directly in withLoading and pactaClient. The problem then is that both these functions return something, so you still have to throw and interrupt the flow of code (I originally tried just leaving hanging promises, but our linter informed me that's a really bad idea)

Since we have to throw, we throw a sentinel so that anything further up the chain knows not to render another dialog.

I tested it in a few different cases and confirmed it does what we expect.

frontend/app.vue Outdated
@@ -1,4 +1,5 @@
<script setup lang="ts">
import { isSilent } from '~/lib/error'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: @/lib/error

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lol not sure how I missed that the rest of the codebase does it that way, done

frontend/app.vue Outdated Show resolved Hide resolved
As discussed, this is an alternate approach to #174 for fixing #168. It handles errors directly in `withLoading` and `pactaClient`. The problem then is that both these functions return _something_, so you still have to throw and interrupt the flow of code (I originally tried just leaving hanging promises, but our linter informed me that's a really bad idea)

Since we have to throw, we throw a sentinel so that anything further up the chain knows not to render another dialog.

I tested it in a few different cases and confirmed it does what we expect.
@bcspragu bcspragu merged commit 3bfd0a5 into main Jan 25, 2024
2 checks passed
@bcspragu bcspragu deleted the brandon/err2 branch January 25, 2024 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants