CHE-10540: stop progress on websocket error#10688
CHE-10540: stop progress on websocket error#10688akurinnoy merged 3 commits intoeclipse-che:masterfrom
Conversation
Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>
Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>
akurinnoy
added
kind/bug
| try { | ||
| const response = JSON.parse(xhr.responseText); | ||
| errorMessage = response.message; | ||
| } catch (e) { } |
There was a problem hiding this comment.
should we log error message/response ?
There was a problem hiding this comment.
It is eventually logged here https://github.com/eclipse/che/blob/389e259d7672fe57300f10ad19657076ea7f19f9/workspace-loader/src/index.ts#L145-L146
Here I only tried to get a meaningful error message to reject a workspace getting or starting promise.
workspace-loader/src/index.ts
Outdated
| (message: any) => { | ||
| if (message.error) { | ||
| this.loader.error(message.error); | ||
| reject(`Failed to run the workspace: "${message.error}"`); |
There was a problem hiding this comment.
should we reject with new Error() ?
There was a problem hiding this comment.
Yes, it may be useful, I'll fix this.
workspace-loader/src/index.ts
Outdated
| this.loader.error(message.error); | ||
| reject(`Failed to run the workspace: "${message.error}"`); | ||
| } else if (message.status === 'RUNNING') { | ||
| resolve(); |
There was a problem hiding this comment.
Can we, please, also check "runtime" is there (not null), when we have RUNNING status. If not - reject the promise.
There is usecase, when user tries to open shared workspace, where he has no permissions on runtime.
There was a problem hiding this comment.
sure, I'll add a fixup
There was a problem hiding this comment.
More details here #10687
Maybe we can also handle second use case here when POST /workspace/WS_ID/runtime is failed
There was a problem hiding this comment.
There was a problem hiding this comment.
Cool. I missed it because PR title and even description tell nothing about this change.
Looks like we will be able to close that issue after merge. I can recheck after merge.
There was a problem hiding this comment.
It was another PR #10642 which is already merged.
There was a problem hiding this comment.
I see, thanks.
I did find the given lines in master and you pointed commit from this PR, it's why I thought that it is introduced in this PR =)
benoitf
removed
the
status/code-review
…e-che#10688) * code clean-up Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com> * CHE-10540: stop workspace loading progress on websocket error Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com> * fixup! CHE-10540: stop workspace loading progress on websocket error
6 participants
What does this PR do?
When workspace loader starts a workspace and receives an error on websocket, then error appears and the progress bar hides.
What issues does this PR fix or reference?
fix #10540
Release Notes
N/A - bugfix
Docs PR
N/A - bugfix