Skip to content

fix: Add requestJsonAndCheck204 to properly handle 204 status checks#3461

Open
wavebyrd wants to merge 1 commit intoPyGithub:mainfrom
wavebyrd:fix/handle-204-status-errors
Open

fix: Add requestJsonAndCheck204 to properly handle 204 status checks#3461
wavebyrd wants to merge 1 commit intoPyGithub:mainfrom
wavebyrd:fix/handle-204-status-errors

Conversation

@wavebyrd
Copy link
Copy Markdown

Summary

Methods that check for 204 status now properly raise exceptions for error codes (401, 403, 500, etc.) while still allowing 404 to return False.

Problem

As described in #2760, methods like is_merged() use requestJson and check status == 204. This causes:

  1. False negatives - A PR could be merged but is_merged returns False if credentials are expired
  2. Silent failures - Consumer is not notified that authentication has expired

Solution

Added a new requestJsonAndCheck204() method in Requester.py that:

  • Returns True for status 204 (success)
  • Returns False for status 404 (not found - valid response for "not merged")
  • Raises appropriate exceptions for other error codes (401, 403, 500, etc.)

Changes

  • Added requestJsonAndCheck204() method to Requester
  • Updated example methods to use the new pattern:
    • PullRequest.is_merged()
    • NamedUser.has_in_following()
    • Repository.create_repository_dispatch()

Note

This PR establishes the pattern. There are ~40 other methods that use status == 204 that could be updated similarly. I can update them all if the maintainers approve this approach, or they can be updated incrementally.

Fixes #2760

🤖 Generated with Claude Code

Methods that check for 204 status now properly raise exceptions for
error codes (401, 403, 500, etc.) while still allowing 404 to return
False.

This fixes the issue where methods like is_merged() would return False
instead of raising an exception when authentication fails.

Updated methods:
- PullRequest.is_merged()
- NamedUser.has_in_following()
- Repository.create_repository_dispatch()

The pattern is established - remaining methods can be updated similarly.

Fixes PyGithub#2760

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wavebyrd
Copy link
Copy Markdown
Author

Ping - this fixes 204 status handling in API checks. Ready for review!

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.

Methods checking for 204 status handle 404 incorrectly

1 participant