fix: tolerate a repository license the API cannot serve - #3610
Conversation
GitHub reports a license on the repository payload even when the license endpoint has nothing to serve, for example after a LICENSE file is added and later removed, which leaves the repository classified as other with a null license URL. The data source returned that 404 verbatim and failed the whole read, so a repository that does not exist read further than one with a LICENSE file. Treat a 404 from the license endpoint as no license, matching how a missing repository is already handled in the same function, and keep failing on every other error.
|
👋 Hi, and thank you for this contribution! This repo is maintained by GitHub and community members on a best-effort basis. We'll get to this as soon as we can. You can help us prioritize by joining the discussion on open issues and PRs, sharing details on the changes you need, and reviewing other contributions. 🤖 This is an automated message. |
Match the error handling pattern documented in ARCHITECTURE.md and used throughout the rest of the provider, rather than errors.As with a separately declared target variable.
The unit test drives the read function against an httptest server, which pins the 404, 200 and 500 branches but does not demonstrate that GitHub actually produces the state the fix targets. Create a repository from a license template, delete its LICENSE file and read it back. The repository keeps its `other` classification while the license endpoint answers 404, reproducing the reported failure straight away rather than depending on timing. Keep the unit test as well, since it covers the 500 branch that an acceptance test cannot provoke on demand.
|
Both done, thanks @deiga Added the acceptance test case as "tolerates a license the API cannot serve", following your suggestion. The repo lands in the bad state right after the file is deleted, so no polling or sleeps are needed. Full Also, I kept the unit test instead of replacing it because it also covers the 500 case, which is what stops the fix from swallowing real API errors. An acceptance test can't make GitHub return a 500 on demand. Happy to drop it if you'd rather only have the acceptance test. |
…e-missing-repository-license
deiga
left a comment
deiga
left a comment
There was a problem hiding this comment.
Looks good! Left a discussion to resolve before continuing
There was a problem hiding this comment.
Pull request overview
These provider review instructions are being used. No blocking findings found.
Fixes repository data-source reads when GitHub reports a license but its license endpoint returns 404.
Changes:
- Treats license-endpoint 404 responses as an empty license.
- Adds unit and acceptance regression coverage.
- Documents the empty-license behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
github/data_source_github_repository.go |
Handles unavailable licenses without failing reads. |
github/data_source_github_repository_test.go |
Adds regression and error-path tests. |
github/acc_helpers_repository_test.go |
Adds license deletion test support. |
templates/data-sources/repository.md.tmpl |
Documents license behavior. |
docs/data-sources/repository.md |
Updates generated documentation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The strict linter enables forcetypeassert for newly added lines, so the unchecked assertions on repo_id and repository_license failed CI.
Resolves #2092
Before the change?
LICENSEfile to a repository and later removing it leaves it classified asotherwith a null license URL, whileGET /repos/{owner}/{repo}/licensereturns 404.data.github_repositorycalls the license endpoint wheneverlicenseis non-null and returns any error as-is, so the whole data source fails and you can't read any other attribute of the repository. Reported in [BUG]: github_repository data source crashes with license not found #2092 on versions 5.44.0 through 6.6, usually on repositories people can't easily change.After the change?
Pull request checklist
No schema change, so no migration is needed.
An acceptance test creates a repository from a license template, deletes
LICENSEand reads it back. I confirmed it fails without the fix, with the same error as the issue. A unit test covers the 404, 200 and 500 branches, including that a 500 still fails the read.Does this introduce a breaking change?
Please see our docs on breaking changes to help!
A repository in this state failed the read before, so nothing could have depended on the old behaviour.
repository_licenseis now empty for it, the same as a repository with no license.Co-authored with an AI assistant, as described in the AI use policy in CONTRIBUTING.md. I've reviewed and tested the change myself, including running the acceptance test against a real account and confirming it fails without the fix.