Skip to content
This repository was archived by the owner on Mar 20, 2024. It is now read-only.

Assertion errors should report a broken state to the relevant PR #171

Open
jdm opened this issue Sep 6, 2018 · 4 comments
Open

Assertion errors should report a broken state to the relevant PR #171

jdm opened this issue Sep 6, 2018 · 4 comments

Comments

@jdm
Copy link
Member

jdm commented Sep 6, 2018

I was left scratching my head when homu stopped making any progress while there was still an ongoing queue of approved PRs. It turns out in servo/servo#21586 that I had given an r+, but homu's state wasn't correct and it stored the approved hash as a previous revision. When it tried to process the queue entry, it yielded this assertion:

INFO:homu:Starting build of servo/servo#21325 on auto: 2b27d4d1ba327c3c4075b22b9dd959642166eee5
Traceback (most recent call last):
  File "/home/servo/homu/_venv/bin/bottle.py", line 862, in _handle
    return route.call(**args)
  File "/home/servo/homu/_venv/bin/bottle.py", line 1740, in wrapper
    rv = callback(*a, **ka)
  File "/home/servo/homu/_venv/lib/python3.4/site-packages/homu/server.py", line 688, in buildbot
    state, logger, repo_cfg)
  File "/home/servo/homu/_venv/lib/python3.4/site-packages/homu/server.py", line 589, in report_build_res
    g.queue_handler()
  File "/home/servo/homu/_venv/lib/python3.4/site-packages/homu/main.py", line 1552, in queue_handler
    return process_queue(states, repos, repo_cfgs, logger, buildbot_slots, db, git_cfg)  # noqa
  File "/home/servo/homu/_venv/lib/python3.4/site-packages/homu/main.py", line 1203, in process_queue
    logger, db, git_cfg):
  File "/home/servo/homu/_venv/lib/python3.4/site-packages/homu/main.py", line 1182, in start_build_or_rebuild
    return start_build(state, repo_cfgs, *args)
  File "/home/servo/homu/_venv/lib/python3.4/site-packages/homu/main.py", line 1000, in start_build
    assert state.head_sha == state.get_repo().pull_request(state.num).head.sha
AssertionError

But I had to go look in the server logs to find that out, which isn't a great failure mode.

@Manishearth
Copy link
Member

Is it ever possible for one of these to leak secrets?

@jdm
Copy link
Member Author

jdm commented Sep 6, 2018

I don't see how this particular assertion could.

@Manishearth
Copy link
Member

Oh, definitely, I'm wary of introducing a generic exception handler somewhere and having it post comments without properly sanitizing what it posts

We can definitely include the backtrace and the error type, but further error details may be tricky?

@jdm
Copy link
Member Author

jdm commented Sep 6, 2018

Well, even a handler that posts "Go look in the log file" comment would be an improvement.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants