Skip to content

Commit 7c6b8d7

Browse files
newstlerclaude
andauthored
fix(github-sync): retry transient network errors during GraphQL batch (#144)
GitHub's GraphQL endpoint occasionally closes connections mid-response, raising EOFError (and siblings like Errno::ECONNRESET / OpenSSL::SSL::SSLError) out of Net::HTTP. The retry loop in User::GithubSyncable#github_graphql_request only caught Net::OpenTimeout/Net::ReadTimeout, so any connection reset escaped the rescue and failed the entire batch of users without retrying — observed in production as EOFError "end of file reached" from UpdateGithubDataJob. Extract the full set of transient network error classes into TRANSIENT_NETWORK_ERRORS and rescue them uniformly so they trigger the existing backoff/retry instead of aborting the batch. Added regression tests covering both paths: retry-then-succeed and retries-exhausted. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 2843e81 commit 7c6b8d7

2 files changed

Lines changed: 68 additions & 2 deletions

File tree

app/models/concerns/user/github_syncable.rb

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,26 @@
11
require "net/http"
22
require "json"
3+
require "openssl"
4+
require "socket"
35

46
module User::GithubSyncable
57
extend ActiveSupport::Concern
68

79
GITHUB_GRAPHQL_ENDPOINT = "https://api.github.com/graphql"
810

11+
TRANSIENT_NETWORK_ERRORS = [
12+
Net::OpenTimeout,
13+
Net::ReadTimeout,
14+
Net::WriteTimeout,
15+
EOFError,
16+
IOError,
17+
SocketError,
18+
Errno::ECONNRESET,
19+
Errno::ECONNREFUSED,
20+
Errno::EPIPE,
21+
OpenSSL::SSL::SSLError
22+
].freeze
23+
924
# Sync from OAuth callback data
1025
def sync_github_data_from_oauth!(auth_data)
1126
api_token = auth_data.credentials.token
@@ -155,12 +170,12 @@ def github_graphql_request(query, api_token, retries: 3)
155170
else
156171
return { errors: [ "HTTP #{response.code}: #{response.message}" ] }
157172
end
158-
rescue Net::OpenTimeout, Net::ReadTimeout => e
173+
rescue *TRANSIENT_NETWORK_ERRORS => e
159174
if attempt < retries - 1
160175
sleep(2 ** (attempt + 1))
161176
next
162177
else
163-
return { errors: [ "Request timed out: #{e.message}" ] }
178+
return { errors: [ "Network error: #{e.class}: #{e.message}" ] }
164179
end
165180
end
166181
end
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
# frozen_string_literal: true
2+
3+
require "test_helper"
4+
require "webmock/minitest"
5+
6+
class User::GithubSyncableTest < ActiveSupport::TestCase
7+
setup do
8+
WebMock.disable_net_connect!(allow_localhost: true)
9+
@user = users(:user_with_testimonial)
10+
@user.update_columns(username: "octocat")
11+
end
12+
13+
teardown do
14+
WebMock.reset!
15+
WebMock.allow_net_connect!
16+
end
17+
18+
# Skip the exponential backoff so tests run fast.
19+
setup { User.singleton_class.send(:define_method, :sleep) { |_| } }
20+
teardown { User.singleton_class.send(:remove_method, :sleep) }
21+
22+
test "batch_sync_github_data! retries on EOFError and succeeds on second attempt" do
23+
stub_request(:post, User::GithubSyncable::GITHUB_GRAPHQL_ENDPOINT)
24+
.to_raise(EOFError.new("end of file reached"))
25+
.then.to_return(
26+
status: 200,
27+
body: {
28+
data: {
29+
user_0: { login: "octocat", name: "The Octocat", email: nil, bio: nil, company: nil, websiteUrl: nil, twitterUsername: nil, location: nil, avatarUrl: "https://example.com/a.png" },
30+
repos_0: { nodes: [] }
31+
}
32+
}.to_json
33+
)
34+
35+
result = User.batch_sync_github_data!([ @user ], api_token: "token")
36+
37+
assert_equal 1, result[:updated], result[:errors].inspect
38+
assert_equal 0, result[:failed]
39+
end
40+
41+
test "batch_sync_github_data! returns a network error after exhausting retries" do
42+
stub_request(:post, User::GithubSyncable::GITHUB_GRAPHQL_ENDPOINT)
43+
.to_raise(EOFError.new("end of file reached"))
44+
45+
result = User.batch_sync_github_data!([ @user ], api_token: "token")
46+
47+
assert_equal 0, result[:updated]
48+
assert_equal 1, result[:failed]
49+
assert_match(/EOFError/, result[:errors].first.to_s)
50+
end
51+
end

0 commit comments

Comments
 (0)