Skip to content

Commit

Permalink
Add tests and improve error messages on Windows (#192)
Browse files Browse the repository at this point in the history
* Add windows to the version matrix

* Check if Process supports forking on this platform before we test term on timeout.

* Improve error message for platforms where term on timeout is unsupported.

* Add test for platforms where term on timeout is unsupported.

* Update error message to include JVM and other platforms.

* Update changelog.
  • Loading branch information
mootpointer authored Aug 12, 2022
1 parent 00d71cc commit 0f35a85
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 14 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ on:

jobs:
test:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
os: [ ubuntu-latest, windows-latest ]
ruby:
- '2.2'
- '2.3'
Expand All @@ -20,6 +20,7 @@ jobs:
- '3.0'
- '3.1'
- 'head'
runs-on: ${{ matrix.os }}
steps:
- name: Checkout code
uses: actions/checkout@v3
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
## HEAD (unreleased)
- Improve message when Terminate on Timeout is used on a platform that does not support it (eg. Windows or JVM)

## 0.6.3

Expand Down
10 changes: 8 additions & 2 deletions lib/rack/timeout/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,14 @@ def initialize(app, service_timeout:nil, wait_timeout:nil, wait_overtime:nil, se
@service_past_wait = service_past_wait == "not_specified" ? ENV.fetch("RACK_TIMEOUT_SERVICE_PAST_WAIT", false).to_s != "false" : service_past_wait

Thread.main['RACK_TIMEOUT_COUNT'] ||= 0
if @term_on_timeout
raise "Current Runtime does not support processes" unless ::Process.respond_to?(:fork)
if @term_on_timeout && !::Process.respond_to?(:fork)
raise(NotImplementedError, <<-MSG)
The platform running your application does not support forking (i.e. Windows, JVM, etc).
To avoid this error, either specify RACK_TIMEOUT_TERM_ON_TIMEOUT=0 or
leave it as default (which will have the same result).
MSG
end
@app = app
end
Expand Down
34 changes: 23 additions & 11 deletions test/env_settings_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,6 @@

class EnvSettingsTest < RackTimeoutTest

def test_service_timeout
with_env(RACK_TIMEOUT_SERVICE_TIMEOUT: 1) do
assert_raises(Rack::Timeout::RequestTimeoutError) do
get "/sleep"
end
end
end

def test_zero_wait_timeout
with_env(RACK_TIMEOUT_WAIT_TIMEOUT: 0) do
Expand All @@ -17,10 +10,29 @@ def test_zero_wait_timeout
end
end

def test_term
with_env(RACK_TIMEOUT_TERM_ON_TIMEOUT: 1) do
assert_raises(SignalException) do
get "/sleep"

if Process.respond_to?(:fork) # This functionality does not work on windows, so we cannot test it there.
def test_service_timeout
with_env(RACK_TIMEOUT_SERVICE_TIMEOUT: 1) do
assert_raises(Rack::Timeout::RequestTimeoutError) do
get "/sleep"
end
end
end

def test_term
with_env(RACK_TIMEOUT_TERM_ON_TIMEOUT: 1) do
assert_raises(SignalException) do
get "/sleep"
end
end
end
else
def test_service_timeout # Confirm that on Windows we raise an exception when someone attempts to use term on timeout
with_env(RACK_TIMEOUT_TERM_ON_TIMEOUT: 1) do
assert_raises(NotImplementedError) do
get "/"
end
end
end
end
Expand Down

0 comments on commit 0f35a85

Please sign in to comment.