Skip to content

Commit

Permalink
Merge pull request #81 from Fullscreen/require-time
Browse files Browse the repository at this point in the history
Bugfix: otherwise Time.parse method returns error
  • Loading branch information
kangkyu committed Apr 20, 2017
2 parents b26c25e + daa5a8e commit a49969f
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 16 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ For more information about changelogs, check
[Keep a Changelog](http://keepachangelog.com) and
[Vandamme](http://tech-angels.github.io/vandamme).

## 0.2.19 - 2017/04/20

* [IMPROVEMENT] Retry 3 times after a server error or a socket error, to fetch
data even with any temporary issue from Facebook.

## 0.2.18 - 2017/04/19

* [ENHANCEMENT] `Funky::Page#videos` now fetches more videos by time-based pagination.
Expand Down
4 changes: 3 additions & 1 deletion lib/funky/connections/api.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
require 'funky/connections/base'
require 'json'
require 'time'

require 'funky/connections/base'

module Funky
# @api private
Expand Down
27 changes: 19 additions & 8 deletions lib/funky/connections/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,34 @@ def self.get_http_request(uri)
Net::HTTP::Get.new(uri.request_uri)
end

def self.response_for(http_request, uri)
def self.response_for(http_request, uri, max_retries = 3)
response = Net::HTTP.start(uri.host, 443, use_ssl: true) do |http|
http.request http_request
end
response
if response.is_a? Net::HTTPSuccess
response
elsif response.is_a? Net::HTTPServerError
sleep_and_retry_response_for(http_request, uri, max_retries, response.body)
else
raise ContentNotFound, "Error #{response.code}: #{response.body}"
end
rescue SocketError => e
raise ConnectionError, e.message
sleep_and_retry_response_for(http_request, uri, max_retries, e.message)
end

def self.json_for(uri)
response = response_for(get_http_request(uri), uri)
if response.code == '200'
JSON.parse(response.body, symbolize_names: true)
def self.sleep_and_retry_response_for(http_request, uri, retries, message)
if retries > 0
sleep (4 - retries) ** 2
response_for http_request, uri, retries - 1
else
raise ContentNotFound, "Error #{response.code}: #{response.body}"
raise ConnectionError, message
end
end

def self.json_for(uri, max_retries = 3)
response = response_for(get_http_request(uri), uri)
JSON.parse(response.body, symbolize_names: true)
end
end
end
end
2 changes: 1 addition & 1 deletion lib/funky/version.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module Funky
VERSION = "0.2.18"
VERSION = "0.2.19"
end
26 changes: 26 additions & 0 deletions spec/pages/videos_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,31 @@
expect(videos.map {|v| v.id}).to include '68196585394'
end
end

context 'given a request that raises' do
let(:response) { Net::HTTPServerError.new nil, nil, nil }
let(:response_body) { '{}' }
before { expect(Net::HTTP).to receive(:start).once.and_return response }
before { allow(response).to receive(:body).and_return response_body }

context 'a 500 Server Error' do
let(:page_id) { fullscreen_page_id }
let(:retry_response) { retry_response_class.new nil, nil, nil }
before { allow(retry_response).to receive(:body).and_return response_body }
before { expect(Net::HTTP).to receive(:start).at_least(:once).and_return retry_response }

context 'every time' do
let(:retry_response_class) { Net::HTTPServerError }

it { expect{ page }.to raise_error Funky::ConnectionError }
end

context 'but returns a success code 2XX the second time' do
let(:retry_response_class) { Net::HTTPOK }

it { expect{ page }.not_to raise_error }
end
end
end
end
end
24 changes: 19 additions & 5 deletions spec/pages/where_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,27 @@
end
end

context 'given a connection error' do
let(:page_ids) { [existing_page_id, another_page_id] }
let(:socket_error) { SocketError.new }
context 'given a request that raises' do
before { expect(Net::HTTP).to receive(:start).once.and_raise http_error }

context 'a SocketError' do
let(:page_ids) { [existing_page_id, another_page_id] }
let(:http_error) { SocketError.new }

context 'every time' do
before { expect(Net::HTTP).to receive(:start).at_least(:once).and_raise http_error }

before { expect(Net::HTTP).to(receive(:start).and_raise socket_error) }
it { expect { pages }.to raise_error(Funky::ConnectionError) }
end

it { expect { pages }.to raise_error(Funky::ConnectionError) }
context 'but works the second time' do
before { expect(Net::HTTP).to receive(:start).at_least(:once).and_return retry_response }
before { allow(retry_response).to receive(:body) }
let(:retry_response) { Net::HTTPOK.new nil, nil, nil }

it { expect { pages }.not_to raise_error }
end
end
end
end
end
2 changes: 1 addition & 1 deletion spec/videos/video_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
let(:video_ids) { [existing_video_id, another_video_id] }
let(:socket_error) { SocketError.new }

before { expect(Net::HTTP).to(receive(:start).and_raise socket_error) }
before { expect(Net::HTTP).to(receive(:start).at_least(1).times.and_raise socket_error) }

it { expect { videos }.to raise_error(Funky::ConnectionError) }
end
Expand Down

0 comments on commit a49969f

Please sign in to comment.