Skip to content

Commit

Permalink
Fix broken handling of signals
Browse files Browse the repository at this point in the history
Our tooling both past (Upstart?) and present (Kubernetes) sends a
`SIGTERM` signal to notify a process that it should gracefully shut
down. At the moment, this library doesn't properly handle this - it does
rescue the error, but does so in the wrong place (so it's not actually
handled) and indiscriminately exits with a non-zero exit code.

- Ensure `SignalException`s raised in `Bunny::Queue#subscribe` (rather
  than the block passed to it) are rescued
- Exit cleanly when receiving a `SignalException` (the comment next to
  `exit(1)` used to say "ensure rabbitmq requeues outstanding messages",
  but that has nothing to do with the exit code, it happens by default
  if the message hasn't been acked by then)
- Add test for signal handling

Ideally in the future we might take this further and allow in-flight
processing to complete before shutting down.
  • Loading branch information
csutter committed Sep 19, 2023
1 parent a820282 commit 407c603
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 12 deletions.
6 changes: 3 additions & 3 deletions lib/govuk_message_queue_consumer/consumer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@ def run(subscribe_opts: {})
@statsd_client.increment("#{@queue_name}.started")
message_consumer.process(message)
@statsd_client.increment("#{@queue_name}.#{message.status}")
rescue SignalException => e
@logger.error "SignalException in processor: \n\n #{e.class}: #{e.message}\n\n#{e.backtrace.join("\n")}"
exit(1) # Ensure rabbitmq requeues outstanding messages
rescue StandardError => e
@statsd_client.increment("#{@queue_name}.uncaught_exception")
GovukError.notify(e) if defined?(GovukError)
@logger.error "Uncaught exception in processor: \n\n #{e.class}: #{e.message}\n\n#{e.backtrace.join("\n")}"
exit(1) # Ensure rabbitmq requeues outstanding messages
end
rescue SignalException => e
@logger.error "SignalException in processor: \n\n #{e.class}: #{e.message}\n\n#{e.backtrace.join("\n")}"
exit
end

private
Expand Down
24 changes: 15 additions & 9 deletions spec/govuk_message_queue_consumer/consumer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,23 @@
let(:client_processor) { instance_double("Client::Processor") }

describe "#run" do
it "doesn't create the queue" do
stubs = create_bunny_stubs
channel = stubs.channel
let(:stubs) { create_bunny_stubs }
let(:channel) { stubs.channel }
let(:queue) { stubs.queue }

it "doesn't create the queue" do
expect(channel).to receive(:queue).with("some-queue", { no_declare: true })

described_class.new(queue_name: "some-queue", processor: client_processor, rabbitmq_connection: stubs.connection, logger: logger).run
end

it "doesn't bind the queue" do
stubs = create_bunny_stubs
queue = stubs.queue

expect(queue).not_to receive(:bind)

described_class.new(queue_name: "some-queue", processor: client_processor, rabbitmq_connection: stubs.connection, logger: logger).run
end

it "calls the heartbeat processor when subscribing to messages" do
stubs = create_bunny_stubs
queue = stubs.queue

expect(queue).to receive(:subscribe).and_yield(:delivery_info_object, :headers, "payload")
heartbeat_processor_stub = instance_double(GovukMessageQueueConsumer::HeartbeatProcessor)
allow(GovukMessageQueueConsumer::HeartbeatProcessor).to receive(:new).and_return(heartbeat_processor_stub)
Expand All @@ -38,5 +33,16 @@

described_class.new(queue_name: "some-queue", processor: client_processor, rabbitmq_connection: stubs.connection, logger: logger).run
end

context "when a SignalException is raised due to the process being terminated" do
before do
allow(logger).to receive(:error)
allow(queue).to receive(:subscribe).and_raise(SignalException, "SIGTERM")
end

it "gracefully exits" do
expect { described_class.new(queue_name: "some-queue", processor: client_processor, rabbitmq_connection: stubs.connection, logger: logger).run }.to raise_error(SystemExit)
end
end
end
end

0 comments on commit 407c603

Please sign in to comment.