Skip to content

Commit

Permalink
Merge pull request #41 from Shopify/remove-default-middlewares
Browse files Browse the repository at this point in the history
Remove default middlewares
  • Loading branch information
casperisfine committed Apr 4, 2023
2 parents e628b50 + daddb1e commit 152aa33
Show file tree
Hide file tree
Showing 8 changed files with 10 additions and 74 deletions.
4 changes: 0 additions & 4 deletions docs/CONFIGURATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,6 @@ Make sure to read the [fork safety guide](FORK_SAFETY.md) before enabling refork

## Rack Features

### `default_middleware`

Sets whether to add Pitchfork's default middlewares. Defaults to `true`.

### `early_hints`

Sets whether to enable the proposed early hints Rack API. Defaults to `false`.
Expand Down
7 changes: 0 additions & 7 deletions exe/pitchfork
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ require 'optparse'
ENV["RACK_ENV"] ||= "development"
rackup_opts = Pitchfork::Configurator::RACKUP
options = rackup_opts[:options]
set_no_default_middleware = true

op = OptionParser.new("", 24, ' ') do |opts|
cmd = File.basename($0)
Expand Down Expand Up @@ -59,11 +58,6 @@ op = OptionParser.new("", 24, ' ') do |opts|
ENV["RACK_ENV"] = e
end

opts.on("-N", "--no-default-middleware",
"do not load middleware implied by RACK_ENV") do |e|
rackup_opts[:no_default_middleware] = true if set_no_default_middleware
end

opts.on("-s", "--server SERVER",
"this flag only exists for compatibility") do |s|
warn "-s/--server only exists for compatibility with rackup"
Expand Down Expand Up @@ -101,7 +95,6 @@ op = OptionParser.new("", 24, ' ') do |opts|
opts.parse! ARGV
end

set_no_default_middleware = false
app = Pitchfork.builder(ARGV[0] || 'config.ru', op)
op = nil

Expand Down
35 changes: 5 additions & 30 deletions lib/pitchfork.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,40 +57,15 @@ def self.builder(ru, op)
Object.const_get(File.basename(ru, '.rb').capitalize)
end

if $DEBUG
require 'pp'
pp({ :inner_app => inner_app })
end

return inner_app unless server.default_middleware

middleware = { # order matters
ContentLength: nil,
Chunked: nil,
CommonLogger: [ $stderr ],
ShowExceptions: nil,
Lint: nil,
TempfileReaper: nil,
}

# return value, matches rackup defaults based on env
# Pitchfork does not support persistent connections, but Rainbows!
# and Zbatery both do. Users accustomed to the Rack::Server default
# middlewares will need ContentLength/Chunked middlewares.
case ENV["RACK_ENV"]
when "development"
when "deployment"
middleware.delete(:ShowExceptions)
middleware.delete(:Lint)
Rack::Builder.new do
use(Rack::Lint)
run inner_app
end.to_app
else
return inner_app
inner_app
end
Rack::Builder.new do
middleware.each do |m, args|
use(Rack.const_get(m), *args) if Rack.const_defined?(m)
end
run inner_app
end.to_app
end
end

Expand Down
7 changes: 0 additions & 7 deletions lib/pitchfork/configurator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,6 @@ def load(merge_defaults = true) #:nodoc:

RACKUP[:set_listener] and
set[:listeners] << "#{RACKUP[:host]}:#{RACKUP[:port]}"

RACKUP[:no_default_middleware] and
set[:default_middleware] = false
end

def commit!(server, options = {}) #:nodoc:
Expand Down Expand Up @@ -149,10 +146,6 @@ def worker_processes(nr)
set_int(:worker_processes, nr, 1)
end

def default_middleware(bool)
set_bool(:default_middleware, bool)
end

def early_hints(bool)
set_bool(:early_hints, bool)
end
Expand Down
1 change: 0 additions & 1 deletion lib/pitchfork/http_server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ def initialize(app, options = {})
@app = app
@respawn = false
@last_check = time_now
@default_middleware = true
@promotion_lock = Flock.new("pitchfork-promotion")

options = options.dup
Expand Down
20 changes: 0 additions & 20 deletions test/integration/t0300-no-default-middleware.sh

This file was deleted.

2 changes: 1 addition & 1 deletion test/integration/test_broken_app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class BrokenAppTest < Pitchfork::IntegrationTest
def test_graceful_handling_of_broken_apps
addr, port = unused_port

pid = spawn_server("-E", "none", app: File.join(ROOT, "test/integration/broken-app.ru"), config: <<~CONFIG,)
pid = spawn_server("-E", "none", app: File.join(ROOT, "test/integration/broken-app.ru"), lint: false, config: <<~CONFIG,)
listen "#{addr}:#{port}"
CONFIG

Expand Down
8 changes: 4 additions & 4 deletions test/integration_test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -156,14 +156,14 @@ def healthy?(host)
false
end

def spawn_server(*args, app:, config:)
def spawn_server(*args, app:, config:, lint: true)
File.write("pitchfork.conf.rb", config)
spawn(BIN, app, "-c", "pitchfork.conf.rb", *args)
env = lint ? { "RACK_ENV" => "development" } : {}
spawn(env, BIN, app, "-c", "pitchfork.conf.rb", *args)
end

def spawn(*args)
env = args.first.is_a?(Hash) ? args.unshift : {}
pid = Process.spawn(env, *args, out: "stdout.log", err: "stderr.log")
pid = Process.spawn(*args, out: "stdout.log", err: "stderr.log")
@_pids << pid
pid
end
Expand Down

0 comments on commit 152aa33

Please sign in to comment.