Closed
Bug 1149176
Opened 10 years ago
Closed 10 years ago
Errors from expired sessions don't bubble up to prevent the action
Categories
(MozReview Graveyard :: General, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mconley, Unassigned)
References
Details
Attachments
(1 file, 1 obsolete file)
jimm gave me a review on a child commit here:
https://reviewboard.mozilla.org/r/6163/
I expected that review to have its comments mirrored to the bug (bug 1148012).
But no mirroring occurred.
Assignee | ||
Comment 1•10 years ago
|
||
Aha, I see this in the log. It *is* due to expired tokens. Gr. We should fix this to actually stop the publishing from happening; even with API keys, we want to detect when one has been invalidated.
14:53:45 ERROR
- Error when calling <function on_review_publishing at 0x7f008f16db18> from SignalHook: Publish error: Bugzilla error: The cookies or token provide were not valid or have expired. You may login again to get new cookies or a new token.
Traceback (most recent call last):
File "/data/www/reviewboard.mozilla.org/venv/lib/python2.6/site-packages/Djblets-0.8.15-py2.6.egg/djblets/extensions/hooks.py", line 196, in _wrap_callback
self.callback(extension=self.extension, **kwargs)
File "build/bdist.linux-x86_64/egg/rbbz/extension.py", line 180, in _transform_errors
raise PublishError('Bugzilla error: %s' % e.msg)
PublishError: Publish error: Bugzilla error: The cookies or token provide were not valid or have expired. You may login again to get new cookies or a new token.
Assignee | ||
Updated•10 years ago
|
Priority: -- → P1
Reporter | ||
Comment 2•10 years ago
|
||
Is the workaround for this to log out of Review Board and log back in?
Assignee | ||
Comment 3•10 years ago
|
||
Yes.
Assignee | ||
Comment 5•10 years ago
|
||
Better summary of the problem. We also have a very easy fix! \o/
Assignee: nobody → mcote
Status: NEW → ASSIGNED
Summary: Some comments not getting mirrored to Bugzilla → Errors from expired sessions don't bubble up to prevent the action
Assignee | ||
Comment 6•10 years ago
|
||
/r/7009 - Bug 1149176 - Don't sandbox errors in hooks that talk to Bugzilla. r=smacleod
Pull down this commit:
hg pull -r 4bb3af0573898f0b2e2810466c3f418bf80d89dd https://reviewboard-hg.mozilla.org/version-control-tools/
Attachment #8591848 -
Flags: review?(smacleod)
Comment 7•10 years ago
|
||
https://reviewboard.mozilla.org/r/7009/#review5817
::: pylib/rbbz/rbbz/extension.py
(Diff revision 1)
> SignalHook(self, review_request_closed,
> - on_review_request_closed_discarded)
> + on_review_request_closed_discarded,
> + sandbox_errors=False)
I don't think this is an abortable signal, we want the sandboxing here.
::: pylib/rbbz/rbbz/extension.py
(Diff revision 1)
> + # Any signal hooks that talk to Bugzilla should have
> + # sandbox_errors=False, since we don't want to complete the action if
any "abortable" signal maybe?
Updated•10 years ago
|
Attachment #8591848 -
Flags: review?(smacleod)
Assignee | ||
Comment 8•10 years ago
|
||
https://reviewboard.mozilla.org/r/7009/#review5821
> I don't think this is an abortable signal, we want the sandboxing here.
Ah shoot, you're right. I think we *should* be using an abortable signal here, but that's a separate bug.
Assignee | ||
Comment 9•10 years ago
|
||
Comment on attachment 8591848 [details]
MozReview Request: bz://1149176/mcote
/r/7009 - Bug 1149176 - Don't sandbox errors in hooks that talk to Bugzilla. r=smacleod
Pull down this commit:
hg pull -r a58f73fba8c2a11d10446f8cbc71603d391148df https://reviewboard-hg.mozilla.org/version-control-tools/
Attachment #8591848 -
Flags: review?(smacleod)
Comment 10•10 years ago
|
||
https://reviewboard.mozilla.org/r/7009/#review5919
::: pylib/rbbz/rbbz/extension.py
(Diff revision 2)
> +
Get rid of this blank line.
Comment 11•10 years ago
|
||
Comment on attachment 8591848 [details]
MozReview Request: bz://1149176/mcote
https://reviewboard.mozilla.org/r/7007/#review5921
Fix it, then Ship-it!
Attachment #8591848 -
Flags: review?(smacleod) → review+
Assignee | ||
Comment 12•10 years ago
|
||
http://hg.mozilla.org/hgcustom/version-control-tools/rev/a6dc54687ced
This should be deployed to production early next week, hopefully on Monday.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 13•10 years ago
|
||
Is there a bug on file to find some way of refreshing the token to avoid this problem entirely?
Flags: needinfo?(mcote)
Assignee | ||
Comment 14•10 years ago
|
||
Not really, since you have to log in with your password, and we don't store that.
However, we will be switching to API keys once bug 1144468 is fixed to allow easy authorization of MozReview, and those won't expire (unless the user explicitly invalidates it). So that should solve the problem properly.
Flags: needinfo?(mcote)
Assignee | ||
Comment 15•10 years ago
|
||
Attachment #8591848 -
Attachment is obsolete: true
Attachment #8619913 -
Flags: review+
Assignee | ||
Comment 16•10 years ago
|
||
Updated•9 years ago
|
Product: Developer Services → MozReview
You need to log in
before you can comment on or make changes to this bug.
Description
•