-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[server] better handle workspacePageClose endpoint response status and log ctx #11817
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
started the job as gitpod-build-hw-hb-s-2.1 because the annotations in the pull request description changed |
ed52e6e
to
87770c7
Compare
@@ -291,33 +292,29 @@ export class UserController { | |||
router.post( | |||
"/auth/workspacePageClose/:instanceID", | |||
async (req: express.Request, res: express.Response, next: express.NextFunction) => { | |||
const logCtx: LogContext = { instanceId: req.params.instanceID }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧡
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes LGTM! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
In previous PR, we did not handle the json-rpc response status #11643 (comment) of server instance
and sharing workspace access #11643 (comment), this PR will handle it and add proper log context #11643 (comment)We removed
accessGuard
of sharing, see reason in How to test sectionRelated Issue(s)
Fixes #
How to test
Check if it will process with sharing workspace or notNo need to test with sharing, since sharing workspace have no update permission on instance which server sendHeartbeat required. This is as expected.
gitpod/components/server/src/auth/resource-access.ts
Lines 265 to 276 in c18dea6
gitpod/components/server/src/workspace/gitpod-server-impl.ts
Line 885 in c18dea6
Can't test with
response status code
since all errors thrown in send heartbeat ends with "does not exist".gitpod/components/server/src/workspace/gitpod-server-impl.ts
Line 905 in c18dea6
Release Notes
Documentation
Werft options: