post_upload.py errors aren't caught

RESOLVED FIXED

Status

P2
normal
RESOLVED FIXED
10 years ago
5 years ago

People

(Reporter: nthomas, Assigned: bhearsum)

Tracking

({fixed1.9.1})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
Hit an issue today where I ran out of space on staging-stage and post_upload.py errored out:
/tools/python/bin/python ../../build/upload.py --base-path ../../dist "../../dist/linux-i686/fr/firefox-3.5rc1.tar.bz2" ../../dist/linux-i686/xpi/fr.xpi
Uploading /builds/moz2_debug_slave/linux_repack/build/mozilla-1.9.1/dist/linux-i686/fr/firefox-3.5rc1.tar.bz2

firefox-3.5rc1.tar.bz2                          0%    0     0.0KB/s   --:-- ETA
firefox-3.5rc1.tar.bz2                        100% 9619KB   9.4MB/s   00:01    
Uploading /builds/moz2_debug_slave/linux_repack/build/mozilla-1.9.1/dist/linux-i686/xpi/fr.xpi

fr.xpi                                          0%    0     0.0KB/s   --:-- ETA
fr.xpi                                        100%  125KB 125.2KB/s   00:00    
Running post-upload command: post_upload.py -p firefox -v 3.5rc1 -n 1 --release-to-candidates-dir
Traceback (most recent call last):
  File "/usr/bin/post_upload.py", line 243, in ?
    func(options, upload_dir, files)
  File "/usr/bin/post_upload.py", line 129, in ReleaseToCandidatesDir
    CopyFileToDir(f, upload_dir, realCandidatesPath, preserve_dirs=True)
  File "/usr/bin/post_upload.py", line 34, in CopyFileToDir
    os.makedirs(full_dest_dir, 0755)
  File "/usr/lib/python2.3/os.py", line 154, in makedirs
    mkdir(name, mode)
OSError: [Errno 28] No space left on device: '/home/ftp/pub/firefox/nightly/3.5rc1-candidates/build1/linux-i686/fr'
Encountered error while uploading

The exit code isn't caught, it goes on to try to upload a second file, and the buildbot step has exit status 0. At least for releases we need to handle this better. I think the culprit is no error handling in hg:build/tools/stage/post_upload.py.

Comment 1

10 years ago
Hm. Correct me if I'm wrong, but it looks like shutil.copyfile() doesn't do any sort of checking.  We could potentially do a checksum of each file pair (old vs new) to CopyFileToDir(), but this will add time, especially if we checksum large files.  A filesize comparison would be faster though less thorough.

Comment 2

10 years ago
I think the problem is in mozilla's build/upload.py, where it doesn't check the return from DoSSHCommand.
It does check the return (DoSSHCommand throws on non-zero exit):
http://mxr.mozilla.org/mozilla-central/source/build/upload.py#122
but upload.py doesn't exit with an error on exceptions:
http://mxr.mozilla.org/mozilla-central/source/build/upload.py#229
(Assignee)

Comment 4

10 years ago
(In reply to comment #3)
> It does check the return (DoSSHCommand throws on non-zero exit):
> http://mxr.mozilla.org/mozilla-central/source/build/upload.py#122
> but upload.py doesn't exit with an error on exceptions:
> http://mxr.mozilla.org/mozilla-central/source/build/upload.py#229

So we just need to sys.exit(1) on the except's in the if __name__ == '__main__' block?
Looks like it, yes, assuming post_upload.py also exits with an error.
(Assignee)

Comment 6

10 years ago
Judging from comment#0 where a Traceback is being printed, and post_upload.py not doing try/except when it calls the functions it looks like it exits non-zero. I just tested locally and indeed, when CopyFileToDir() raises an exception the exit code is non-zero. I'll get a patch for upload.py ready.
Assignee: nobody → bhearsum
Priority: -- → P2
(Assignee)

Comment 7

10 years ago
Created attachment 373340 [details] [diff] [review]
use sys.exit(non-zero) when exceptions are caught in upload.py
Attachment #373340 - Flags: review?(ted.mielczarek)
Attachment #373340 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Updated

10 years ago
Attachment #373340 - Flags: checked‑in+ checked‑in+
(Assignee)

Comment 8

10 years ago
Comment on attachment 373340 [details] [diff] [review]
use sys.exit(non-zero) when exceptions are caught in upload.py

changeset:   27534:5ed2092ca3a4

Landed on mozilla-central.
(Assignee)

Comment 9

10 years ago
Comment on attachment 373340 [details] [diff] [review]
use sys.exit(non-zero) when exceptions are caught in upload.py

The codepaths here aren't reached unless there's a failure to upload (eg. network problem). These are pretty rare, so they haven't been hit in a real build yet. It's a pretty trivial patch which will only improve error reporting, so I think this is ready to go on mozilla-1.9.1.

Technically, this is POTB, so I'm requesting approval.
(Assignee)

Updated

10 years ago
Attachment #373340 - Flags: approval1.9.1?
Comment on attachment 373340 [details] [diff] [review]
use sys.exit(non-zero) when exceptions are caught in upload.py

a191=beltzner
Attachment #373340 - Flags: approval1.9.1? → approval1.9.1+
(Assignee)

Comment 11

10 years ago
Comment on attachment 373340 [details] [diff] [review]
use sys.exit(non-zero) when exceptions are caught in upload.py

mozilla-1.9.1: changeset:   24948:376ea3c5e508
(Assignee)

Comment 12

10 years ago
I just landed this on mozilla-1.9.1. Tracemonkey will pick this up next time they merge, so I'm going to consider this bug FIXED.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Updated

10 years ago
Keywords: fixed1.9.1
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.