Fix: display error messages when GitHub installations fail to load#4488
Fix: display error messages when GitHub installations fail to load#4488
Conversation
Previously, when the GitHub installations API call failed (e.g., due to expired OAuth tokens), users would see an empty dropdown with no explanation. This adds proper error handling that displays specific error messages and guides users to reconnect their GitHub account in Settings when needed.
Streamline error handling to focus on the most likely case (expired OAuth tokens) with specific guidance, while keeping other errors simple with helpful debugging info. Removes hypothetical error cases that haven't been observed yet.
Fix HEEx interpolation syntax, add ARIA attributes for accessibility, limit debug output to development environment, and standardize link styling
Add comprehensive test coverage for error banner rendering including OAuth token errors, generic GitHub errors, map format errors, and unknown error fallbacks
Replace .async_result wrapper with conditional error banner to ensure form inputs remain in the DOM but disabled during errors. This maintains compatibility with existing tests and provides better UX by showing users what fields they need to complete once the error is resolved.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4488 +/- ##
==========================================
- Coverage 89.55% 89.52% -0.03%
==========================================
Files 425 425
Lines 20307 20307
==========================================
- Hits 18185 18180 -5
- Misses 2122 2127 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
midigofrank
left a comment
There was a problem hiding this comment.
@lmac-1 I don't think the issue is about tokens, #4467, if it was about tokens then it would not load on the parent project as well.
I love the idea of putting the warning banner when there is an error while fetching installations though. Maybe we could focus it on the 2 error possibilities:
- Github is down
- Access token is expired or user uninstalled the app from github.
There was a "get of jail" link, which would redirect users to github and handle the second scenario:
<div class="mt-2">
Can't find the right installation or repository?
<.link
target="_blank"
class="link"
href={"https://github.com/apps/#{github_config()[:app_name]}"}
>
Create/update GitHub installations or modify permissions.
</.link>
</div>
|
@midigofrank Hey Frank, thank you for the review! A bit of extra context: I verified with Aisha and it's not just happening for sandboxes, it also happens at the parent project level, I left a comment on #4467. Sorry, I should have put it in the additional notes for reviewer here too. When in a huddle for Aisha I also got her to go to GitHub to check installations and it didn't fix things. Laura also raised the same issue on Friday at project-level and I got her to remove and re-add her GitHub in settings and it fixed the drop down issue. So perhaps there are 3 scenarios:
Do you want me to do the 'get out of jail free' link for scenario 3? Also do you think that telling the user to go to their settings to remove and readd their GitHub account it a good thing to do to fix 2? Or is there a better way to do this? |
|
Aaah, that's good to know @lmac-1 . Yes, I think we should have the get out jail link at the top instead of leading them to the profile page. It is worth testing though |
Previously, all GitHub installation fetch errors showed the same generic error message. Now display specific guidance based on the failure type: - Token expired/invalid: Direct user to Settings to reconnect GitHub account - No installations found: Provide link to install GitHub app - Other API errors: Show generic error with refresh suggestion Also fix installations_select_options to properly extract installations list from the result map structure, preventing crashes when rendering the dropdown.
Move complex template conditionals to helper functions following Lightning pattern of keeping logic in component modules rather than templates. - Add helper functions: show_invalid_oauth_error?, show_no_installations_warning?, show_api_error? - Create reusable error_banner component to eliminate 94 lines of duplication - Fix installations_select_options to correctly handle AsyncResult unpacking - Fix syntax error in dev debug output using proper inspect interpolation - Add rel="noopener noreferrer" to external GitHub link for security - Improve dev error display with collapsible details element
A user reported that they have an empty dropdown when trying to add a GitHub installation:
My theory is that an API call has failed (e.g due to expired OAuth tokens), and we don't handle these errors. This PR adds proper error handling that displays specific error messages and guides users to reconnect their GitHub account in Settings when needed.
What this PR does
When the GitHub installations API call fails (e.g., due to expired OAuth tokens), users would see an empty dropdown with no explanation. This PR adds:
role="alert",aria-live="polite") for screen reader supportValidation steps
Not a very easy one to validate/test but UI will look something like this, with the GitHub fields disabled, code error messages only visible on dev environments:
If you really want to test the UI, you can ask Claude to comment out some code so you can see the UI in all conditions.
Additional notes for the reviewer
AI Usage
Please disclose whether you've used AI anywhere in this PR (it's cool, we just
want to know!):
You can read more details in our
Responsible AI Policy
Pre-submission checklist
/reviewwith Claude Code)
(e.g.,
:owner,:admin,:editor,:viewer)