Sandbox reviewboard.hostingsvcs.service.HostingService
Review Request #6652 — Created Nov. 28, 2014 and discarded
Extensions that create a HostingService subclass (using the HostingServiceHook) can throw exceptions inside Reviewboard. The parts of Reviewboard that call those methods have been sandboxed.
Now when a HostingService subclass from an extension throws an exception; Reviewboard logs the errors with enough information to find which method in the HostingService subclass threw the exception.
Unit tests have been written to make sure that functions from a Widget subclass have been called, and when an exception is thrown it gets logged.
The tests fail without the sanboxing, and succeed with it.
| Description | From | Last Updated |
|---|---|---|
|
'SCMTool' imported but unused |
 reviewbot
|
|
|
This will make it fail silently rather than reporting any errors to the user. |
 david
|
|
|
This is still making it fail silently (reporting zero branches). Before it would tell the user about an error occurring. … |
david
|
|
|
Same here. |
david
|
|
|
And here. |
david
|
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 2 (+155 -2) |
-
Tool: Pyflakes Processed Files: reviewboard/scmtools/tests.py reviewboard/hostingsvcs/models.py reviewboard/hostingsvcs/tests.py Tool: PEP8 Style Checker Processed Files: reviewboard/scmtools/tests.py reviewboard/hostingsvcs/models.py reviewboard/hostingsvcs/tests.py -
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 3 (+180 -5) |
-
Tool: Pyflakes Processed Files: reviewboard/scmtools/tests.py reviewboard/hostingsvcs/models.py reviewboard/scmtools/models.py reviewboard/hostingsvcs/tests.py reviewboard/hostingsvcs/service.py Tool: PEP8 Style Checker Processed Files: reviewboard/scmtools/tests.py reviewboard/hostingsvcs/models.py reviewboard/scmtools/models.py reviewboard/hostingsvcs/tests.py reviewboard/hostingsvcs/service.py
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 4 (+237 -7) |
-
Tool: Pyflakes Processed Files: reviewboard/hostingsvcs/models.py reviewboard/scmtools/models.py reviewboard/webapi/resources/remote_repository.py reviewboard/scmtools/tests.py reviewboard/webapi/tests/test_remote_repository.py reviewboard/hostingsvcs/service.py reviewboard/hostingsvcs/tests.py Tool: PEP8 Style Checker Processed Files: reviewboard/hostingsvcs/models.py reviewboard/scmtools/models.py reviewboard/webapi/resources/remote_repository.py reviewboard/scmtools/tests.py reviewboard/webapi/tests/test_remote_repository.py reviewboard/hostingsvcs/service.py reviewboard/hostingsvcs/tests.py
Summary: |
|
|||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
|||||||||||||||||||||
Commit: |
|
|||||||||||||||||||||
Diff: |
Revision 5 (+238 -9) |
-
Tool: Pyflakes Processed Files: reviewboard/hostingsvcs/models.py reviewboard/scmtools/models.py reviewboard/webapi/resources/remote_repository.py reviewboard/scmtools/tests.py reviewboard/scmtools/admin.py reviewboard/webapi/tests/test_remote_repository.py reviewboard/hostingsvcs/service.py reviewboard/hostingsvcs/tests.py Tool: PEP8 Style Checker Processed Files: reviewboard/hostingsvcs/models.py reviewboard/scmtools/models.py reviewboard/webapi/resources/remote_repository.py reviewboard/scmtools/tests.py reviewboard/scmtools/admin.py reviewboard/webapi/tests/test_remote_repository.py reviewboard/hostingsvcs/service.py reviewboard/hostingsvcs/tests.py
-
-
reviewboard/scmtools/models.py (Diff revision 5) This will make it fail silently rather than reporting any errors to the user.
-
-
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 6 (+253 -14) |
-
Tool: Pyflakes Processed Files: reviewboard/hostingsvcs/models.py reviewboard/scmtools/models.py reviewboard/webapi/resources/remote_repository.py reviewboard/scmtools/tests.py reviewboard/scmtools/admin.py reviewboard/webapi/tests/test_remote_repository.py reviewboard/hostingsvcs/service.py reviewboard/hostingsvcs/tests.py Tool: PEP8 Style Checker Processed Files: reviewboard/hostingsvcs/models.py reviewboard/scmtools/models.py reviewboard/webapi/resources/remote_repository.py reviewboard/scmtools/tests.py reviewboard/scmtools/admin.py reviewboard/webapi/tests/test_remote_repository.py reviewboard/hostingsvcs/service.py reviewboard/hostingsvcs/tests.py
-
In your testing done, you say "Unit tests have been written to make sure that functions from a Widget subclass have been called, and when an exception is thrown it gets logged." That doesn't seem correct for this change.
-
reviewboard/scmtools/models.py (Diff revisions 5 - 6) This is still making it fail silently (reporting zero branches). Before it would tell the user about an error occurring.
These methods here should probably not be wrapped at all, because we expect them to throw exceptions and the proper error handling is already in place.
Tool: PEP8 Style Checker Processed Files: reviewboard/hostingsvcs/models.py reviewboard/hostingsvcs/tests.py Tool: Pyflakes Processed Files: reviewboard/hostingsvcs/models.py reviewboard/hostingsvcs/tests.py