From 023a6867fabede9ba6922ab8e222e7c21de2b9b6 Mon Sep 17 00:00:00 2001 From: Andrew Bromwich Date: Thu, 27 Jun 2024 11:15:15 +1000 Subject: [PATCH 1/5] Use Page.setDefaultTimeout with `timeout` option --- .github/workflows/test.yml | 4 ---- lib/grover/js/processor.cjs | 8 +++++-- spec/grover/processor_spec.rb | 41 +++++++++++++++++++++++++++++++---- spec/spec_helper.rb | 9 ++++++-- 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 937c5a3a..c4468608 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -69,13 +69,9 @@ jobs: - name: Run tests run: bundle exec rspec - env: - PUPPETEER_VERSION: ${{ matrix.puppeteer-version }} - name: Run tests with remote browser run: bundle exec rspec --tag remote_browser - env: - PUPPETEER_VERSION: ${{ matrix.puppeteer-version }} - name: Test & publish code coverage uses: paambaati/codeclimate-action@v3.2.0 diff --git a/lib/grover/js/processor.cjs b/lib/grover/js/processor.cjs index 3604f2e4..65f84ff3 100644 --- a/lib/grover/js/processor.cjs +++ b/lib/grover/js/processor.cjs @@ -80,6 +80,12 @@ const _processPage = (async (convertAction, urlOrHtml, options) => { page = await browser.newPage(); + // Apply global page timeout + const timeout = options.timeout; delete options.timeout + if (timeout !== undefined) { + page.setDefaultTimeout(timeout); + } + // Basic auth const username = options.username; delete options.username const password = options.password; delete options.password @@ -100,10 +106,8 @@ const _processPage = (async (convertAction, urlOrHtml, options) => { } // Setup timeout option (if provided) - if (options.timeout === null) delete options.timeout; let requestOptions = {}; let requestTimeout = options.requestTimeout; delete options.requestTimeout; - if (requestTimeout === undefined) requestTimeout = options.timeout; if (requestTimeout !== undefined) { requestOptions.timeout = requestTimeout; } diff --git a/spec/grover/processor_spec.rb b/spec/grover/processor_spec.rb index dd32c562..f1e2c96b 100644 --- a/spec/grover/processor_spec.rb +++ b/spec/grover/processor_spec.rb @@ -773,7 +773,7 @@ <<-HTML - #{' '} +

Loading

@@ -833,16 +833,19 @@ end end - shared_examples 'raises navigation timeout error' do + shared_examples 'raises navigation timeout error' do |timeout: 1| if puppeteer_version_on_or_after? '2.0.0' it do - expect { convert }.to raise_error Grover::JavaScript::TimeoutError, 'Navigation timeout of 1 ms exceeded' + expect { convert }.to raise_error( + Grover::JavaScript::TimeoutError, + "Navigation timeout of #{timeout} ms exceeded" + ) end else it do expect { convert }.to raise_error( Grover::JavaScript::TimeoutError, - 'Navigation Timeout Exceeded: 1ms exceeded' + "Navigation Timeout Exceeded: #{timeout}ms exceeded" ) end end @@ -862,6 +865,36 @@ it { is_expected.to start_with "%PDF-1.4\n" } end + + context 'when the waitForSelector option is provided' do + let(:url_or_html) do + <<-HTML + + + + + + HTML + end + let(:options) { basic_header_footer_options.merge('timeout' => timeout, 'waitForSelector' => 'h1') } + + context 'when the timeout is less than the page content load' do + let(:timeout) { 100 } + + it_behaves_like 'raises navigation timeout error', timeout: 100 + end + + context 'when the timeout is greater than the page content load' do + let(:timeout) { 2000 } + + it { is_expected.to start_with "%PDF-1.4\n" } + it { expect(pdf_text_content).to eq "#{date} Hey there #{protocol}://www.example.net/foo/bar 1/1" } + end + end end context 'when requestTimeout option is specified' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f42dd224..61ec800a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -30,15 +30,20 @@ MiniMagick.validate_on_create = false def puppeteer_version_on_or_after?(version) - puppeteer_version = ENV.fetch('PUPPETEER_VERSION', '') puppeteer_version.empty? || Gem::Version.new(puppeteer_version) >= Gem::Version.new(version) end def puppeteer_version_on_or_before?(version) - puppeteer_version = ENV.fetch('PUPPETEER_VERSION', '') puppeteer_version.empty? || Gem::Version.new(puppeteer_version) <= Gem::Version.new(version) end +def puppeteer_version + @puppeteer_version ||= begin + version = `node -p "require('puppeteer/package.json').version"`.strip + version if version.match?(/\A\d+\.\d+\.\d+\z/) + end +end + def linux_system? uname = `uname -s` uname.start_with? 'Linux' From f2b90441629d0c0207d1948ffeaef004a9d43f9b Mon Sep 17 00:00:00 2001 From: Andrew Bromwich Date: Thu, 27 Jun 2024 12:05:21 +1000 Subject: [PATCH 2/5] Fix lints --- spec/grover/processor_spec.rb | 2 +- spec/spec_helper.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/grover/processor_spec.rb b/spec/grover/processor_spec.rb index f1e2c96b..60d72c29 100644 --- a/spec/grover/processor_spec.rb +++ b/spec/grover/processor_spec.rb @@ -871,7 +871,7 @@ <<-HTML - +