Skip to content

Commit

Permalink
worker: Log error in remote_getWorkerInfo instead of crashing
Browse files Browse the repository at this point in the history
Worker lists and reads all file in WORKER/info folder in functin remote_getWorkerInfo. To prevent it from crashing when it encounteres file that can not be decoded we now only report this error to log and continue.

Fixes buildbot#3585
Fixes buildbot#4758
Fixes buildbot#6932
  • Loading branch information
vibbo authored and pmisik committed Sep 6, 2023
1 parent ede83f4 commit 7fda2fa
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 1 deletion.
1 change: 1 addition & 0 deletions newsfragments/get-worker-info.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix worker not connecting error when there are files in WORKER/info folder that can not be decoded. (:issue:`3585`) (:issue:`4758`) (:issue:`6932`)
8 changes: 7 additions & 1 deletion worker/buildbot_worker/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@
from twisted.application import service
from twisted.internet import defer
from twisted.internet import reactor
from twisted.python import failure
from twisted.python import log
from twisted.spread import pb

import buildbot_worker
from buildbot_worker.commands import base
from buildbot_worker.commands import registry
from buildbot_worker.compat import bytes2unicode
from buildbot_worker.util import buffer_manager
from buildbot_worker.util import lineboundaries

Expand Down Expand Up @@ -218,7 +220,11 @@ def remote_getWorkerInfo(self):
filename = os.path.join(basedir, f)
if os.path.isfile(filename):
with open(filename, "r") as fin:
files[f] = fin.read()
try:
files[f] = bytes2unicode(fin.read())
except UnicodeDecodeError:
log.err(failure.Failure(),
'error while reading file: %s' % (filename))

self._read_os_release(self.os_release_file, files)

Expand Down
35 changes: 35 additions & 0 deletions worker/buildbot_worker/test/unit/test_bot.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,41 @@ def test_getWorkerInfo_nodir(self):
['environ', 'system', 'numcpus', 'basedir', 'worker_commands', 'version',
'delete_leftover_dirs']))

@defer.inlineCallbacks
def test_getWorkerInfo_decode_error(self):
infodir = os.path.join(self.basedir, "info")
os.makedirs(infodir)
with open(os.path.join(infodir, "admin"), "w") as f:
f.write("testy!")
with open(os.path.join(infodir, "foo"), "w") as f:
f.write("bar")
with open(os.path.join(infodir, "environ"), "w") as f:
f.write("something else")
# This will not be part of worker info
with open(os.path.join(infodir, "binary"), "wb") as f:
f.write(b"\x90")

# patch the log.err, otherwise trial will think something *actually*
# failed
self.patch(log, "err", lambda f, x: None)

info = yield self.bot.callRemote("getWorkerInfo")

# remove any os_ fields as they are dependent on the test environment
info = {k: v for k, v in info.items() if not k.startswith("os_")}

self.assertEqual(info, {
"admin": 'testy!',
"foo": 'bar',
"environ": os.environ,
"system": os.name,
"basedir": self.basedir,
"worker_commands": self.real_bot.remote_getCommands(),
"version": self.real_bot.remote_getVersion(),
"numcpus": multiprocessing.cpu_count(),
"delete_leftover_dirs": False
})

def test_shutdown(self):
d1 = defer.Deferred()
self.patch(reactor, "stop", lambda: d1.callback(None))
Expand Down

0 comments on commit 7fda2fa

Please sign in to comment.