Skip to content

Fix: display error messages when GitHub installations fail to load#4488

Open
lmac-1 wants to merge 9 commits intomainfrom
4467-handle-github-sync-errors
Open

Fix: display error messages when GitHub installations fail to load#4488
lmac-1 wants to merge 9 commits intomainfrom
4467-handle-github-sync-errors

Conversation

@lmac-1
Copy link
Collaborator

@lmac-1 lmac-1 commented Mar 4, 2026

A user reported that they have an empty dropdown when trying to add a GitHub installation:

image

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:

  • Error handling: Displays specific error messages for different failure types (invalid OAuth tokens, generic GitHub errors, network errors)
  • User guidance: Provides actionable steps like reconnecting GitHub account in Settings
  • Accessibility: ARIA attributes (role="alert", aria-live="polite") for screen reader support
  • Security: Limits detailed error output to development environment only
  • Test coverage: Comprehensive tests for all error state scenarios

Validation 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:

image image image image

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

  1. The plan for this PR is that once it's merged, I'll get the user to retest and see what error they are getting. Then get them to disconnect and reconnect their GitHub account in order to see if it gets fixed by that.
  2. If we can confirm that the reason that error is happening is because of the expired oauth token, we could look how to make that experience a bit better.

AI Usage

Please disclose whether you've used AI anywhere in this PR (it's cool, we just
want to know!):

  • I have used Claude Code
  • I have used another model
  • I have not used AI

You can read more details in our
Responsible AI Policy

Pre-submission checklist

  • I have performed an AI review of my code (we recommend using /review
    with Claude Code)
  • I have implemented and tested all related authorization policies.
    (e.g., :owner, :admin, :editor, :viewer)
  • I have updated the changelog.
  • I have ticked a box in "AI usage" in this PR

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.
lmac-1 added 5 commits March 6, 2026 13:17
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
Copy link

codecov bot commented Mar 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.52%. Comparing base (5d4762c) to head (ec6f3c3).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lmac-1 lmac-1 marked this pull request as ready for review March 9, 2026 11:37
@lmac-1 lmac-1 requested review from doc-han and elias-ba March 9, 2026 11:43
@theroinaochieng theroinaochieng requested review from midigofrank and removed request for elias-ba March 10, 2026 08:18
Copy link
Collaborator

@midigofrank midigofrank left a comment

Choose a reason for hiding this comment

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

@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:

  1. Github is down
  2. 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>

@github-project-automation github-project-automation bot moved this from New Issues to In review in Core Mar 11, 2026
@lmac-1
Copy link
Collaborator Author

lmac-1 commented Mar 11, 2026

@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:

  1. GitHub is down
  2. Access token is expired (will be fixed by removing and readding GitHub in settings)
  3. User uninstalled the app from GitHub

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?

@midigofrank
Copy link
Collaborator

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

lmac-1 added 2 commits March 13, 2026 16:46
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

2 participants