Skip to content

Commit

Permalink
Return 404 response for missing log file. Closes Supervisor#1406
Browse files Browse the repository at this point in the history
  • Loading branch information
mnaberez committed Mar 13, 2021
1 parent eabc2c5 commit 8bc8a57
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 9 deletions.
6 changes: 6 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
4.3.0.dev0 (Next Release)
-------------------------

- The web interface will now return a 404 Not Found response if a log file
is missing. Previously, it would return 410 Gone. It was changed because
410 is intended to mean that the condition is likely to be permanent. A
log file missing is usually temporary, e.g. a process that was never started
will not have a log file but will have one as soon as it is started.

4.2.2 (2021-02-26)
------------------

Expand Down
14 changes: 9 additions & 5 deletions supervisor/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -737,10 +737,11 @@ def handle_request(self, request):
logfile = getattr(process.config, '%s_logfile' % channel, None)

if logfile is None or not os.path.exists(logfile):
# XXX problematic: processes that don't start won't have a log
# file and we probably don't want to go into fatal state if we try
# to read the log of a process that did not start.
request.error(410) # gone
# we return 404 because no logfile is a temporary condition.
# if the process has never been started, no logfile will exist
# on disk. a logfile of None is also a temporay condition,
# since the config file can be reloaded.
request.error(404) # not found
return

mtime = os.stat(logfile)[stat.ST_MTIME]
Expand Down Expand Up @@ -774,7 +775,10 @@ def handle_request(self, request):
logfile = self.supervisord.options.logfile

if logfile is None or not os.path.exists(logfile):
request.error(410) # gone
# we return 404 because no logfile is a temporary condition.
# even if a log file of None is configured, the config file
# may be reloaded, and the new config may have a logfile.
request.error(404) # not found
return

mtime = os.stat(logfile)[stat.ST_MTIME]
Expand Down
8 changes: 4 additions & 4 deletions supervisor/tests/test_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def test_handle_request_stdout_logfile_none(self):
handler = self._makeOne(supervisord)
request = DummyRequest('/logtail/process1', None, None, None)
handler.handle_request(request)
self.assertEqual(request._error, 410)
self.assertEqual(request._error, 404)

def test_handle_request_stdout_logfile_missing(self):
options = DummyOptions()
Expand All @@ -54,7 +54,7 @@ def test_handle_request_stdout_logfile_missing(self):
handler = self._makeOne(supervisord)
request = DummyRequest('/logtail/foo', None, None, None)
handler.handle_request(request)
self.assertEqual(request._error, 410)
self.assertEqual(request._error, 404)

def test_handle_request(self):
with tempfile.NamedTemporaryFile() as f:
Expand Down Expand Up @@ -84,15 +84,15 @@ def test_handle_request_stdout_logfile_none(self):
handler = self._makeOne(supervisor)
request = DummyRequest('/mainlogtail', None, None, None)
handler.handle_request(request)
self.assertEqual(request._error, 410)
self.assertEqual(request._error, 404)

def test_handle_request_stdout_logfile_missing(self):
supervisor = DummySupervisor()
supervisor.options.logfile = '/not/there'
request = DummyRequest('/mainlogtail', None, None, None)
handler = self._makeOne(supervisor)
handler.handle_request(request)
self.assertEqual(request._error, 410)
self.assertEqual(request._error, 404)

def test_handle_request(self):
supervisor = DummySupervisor()
Expand Down

0 comments on commit 8bc8a57

Please sign in to comment.