Update to 1.71 #5535

fritterhoff · 2022-09-03T07:22:07Z

As a first attempt the required fixes/changes for the code version 1.71.

The telemetry fix required some adjustments. I also included a fix that was recently merged upstream (microsoft/vscode@b108bc8)
The CLI changes seem to be included in the upstream version now. Feel free to remove the remaining changes...

Testing Plan

To test this locally:

run this:

gh pr checkout 5535 && git submodule update --init && quilt push -a && yarn && yarn watch --port 3000

navigate to localhost:3000

Patches tested

The following patches were tested locally:

Fixes #5528

fritterhoff · 2022-09-03T07:30:24Z

patches/telemetry.diff

-				// This is most likely due to ad blockers
-				fetch(telemetryEndpointUrl, { method: 'POST' }).catch(err => {
-					this.impl = NullTelemetryService;
-				});


I actually have no idea what happened with this segment in the original version. I have not found any commit where this was removed or changed. Maybe sbd. finds the reason...

the cli fix seems to be fixed in upstream, the telemtry patch requires (again) some fixing and adjustments.

fritterhoff · 2022-09-03T11:53:53Z

@benz0li feel free to give the CI artificats a try. From my point of view it looks fine.

benz0li · 2022-09-03T12:31:02Z

@fritterhoff code-server-4.6.1-linux-amd64.tar.gz of the artifacts is deployed at https://vscode-r.jupyter.b-data.ch; Image R (base:test) + code-server.

Functionality [modified by patches] tested and found to work:

base-path
cli-window-open
local-storage
- I assume this is about settings
marketplace
proxy-url
service-worker
webview

👉 Same as #5417 (comment).

benz0li · 2022-09-03T12:35:51Z

@fritterhoff Could you please check if rendering is working properly for you at https://vscode-r.jupyter.b-data.ch?:

I get the following warning in the browser's web console:

[Log]  WARN – "Canvas could not be loaded. Falling back to the dom renderer type." (workbench.web.main.js, line 603)
TypeError: Type error
observe
(anonymous function) — DevicePixelObserver.ts:32
e — CanvasRenderer.ts:70
(anonymous function) — CanvasAddon.ts:32
(anonymous function) — AddonManager.ts:34
(anonymous function) — testingOutputPeek.ts:1023
asyncFunctionResume
(anonymous function)
promiseReactionJobWithoutPromise
promiseReactionJob

When I reload the page with an open Terminal, it does not get rendered properly.
ℹ️ I do not get this warning with v1.70.2.

fritterhoff · 2022-09-03T13:13:32Z

I'll check it asap.

Well.. yes the rendering seems to fail. I'll try it with my own containers.

fritterhoff · 2022-09-04T04:52:56Z

So I can at least point to the commit where this change was introduced: microsoft/vscode@2f72682

Have to debug that further 😉

fritterhoff · 2022-09-04T05:22:30Z

I'll check it asap.

Well.. yes the rendering seems to fail. I'll try it with my own containers.

My own containers work fine so maybe there is a bug in your Dockerfile...

fritterhoff · 2022-09-04T06:08:57Z

I'll check it asap.

Well.. yes the rendering seems to fail. I'll try it with my own containers.

So I tried again. Now it works fine, beside a rather slow loading. Also reloading works. I have no idea what actually failed yesterday. Maybe the browser only ran into a timeout or something like that.

benz0li · 2022-09-04T07:49:26Z

Everything works fine with Firefox as it uses the WebGL renderer.

Safari falls back to the dom render, as canvas can not be loaded [and WebGL is ~~not supported~~ disabled for Safari by VS Code].

1.71.0

Screenshot

Log

nothing about xterm

1.70.2

Screenshot

Log

[I 2022-09-04 09:21:24.192 SingleUserLabApp log:189] 200 GET /user/benz0li/code-server/stable-e4503b30fc78200f846c62cf8091b76ff5547662/static/node_modules/xterm/lib/xterm.js (benz0li@[redacted]) 51.26ms
[I 2022-09-04 09:21:24.318 SingleUserLabApp log:189] 200 GET /user/benz0li/code-server/stable-e4503b30fc78200f846c62cf8091b76ff5547662/static/node_modules/xterm-addon-unicode11/lib/xterm-addon-unicode11.js (benz0li@[redacted]) 11.81ms
[I 2022-09-04 09:21:24.427 SingleUserLabApp log:189] 200 GET /user/benz0li/code-server/stable-e4503b30fc78200f846c62cf8091b76ff5547662/static/node_modules/xterm-addon-unicode11/lib/xterm-addon-unicode11.js.map (benz0li@[redacted]) 10.74ms
[I 2022-09-04 09:21:24.539 SingleUserLabApp log:189] 200 GET /user/benz0li/code-server/stable-e4503b30fc78200f846c62cf8091b76ff5547662/static/node_modules/xterm/lib/xterm.js.map (benz0li@[redacted]) 189.38ms

benz0li · 2022-09-04T07:52:02Z

Workaround for Safari: Disable Gpu Acceleration

Settings

Settings (JSON)

"terminal.integrated.gpuAcceleration": "off"

fritterhoff · 2022-09-04T07:52:55Z

Damn it. Sadly I don't have the option to test safari. That will get complicated to reproduce/fix. Can you maybe test codespaces?

fritterhoff · 2022-09-04T08:17:50Z

I only made some quick checks using BrowserStack. Here Safari works fine but I'm not sure if they make any modifications or if I made did not test it enough.

If i remember correct, @jsjoeio is also using a Mac. So maybe he can debug that a little bit more. I don't have access to a Macintosh device at all - sorry.

benz0li · 2022-09-04T08:54:39Z

Safari: Web Console

(The link for xterm-addon-canvas.js:0 points to https://vscode-r.jupyter.b-data.ch/user/benz0li/co…ules/xterm-addon-canvas/lib/xterm-addon-canvas.js)
(ℹ️ There is a … in the URL and it is limited to 100 characters...)
👉 The 100 character limit only seems to apply to displaying certain links in the console.

Safari: Web Network

👉 According to the network pane, xterm-addon-canvas.js is served from memory.

benz0li · 2022-09-04T09:04:15Z

Can you maybe test codespaces?

No, unfortunately not. I am mainly using my self-hosted GitLab instance and a free account on both https://github.com and https://gitlab.com.

benz0li · 2022-09-04T09:39:53Z

So I can at least point to the commit where this change was introduced: microsoft/vscode@2f72682

Well... xterm has been updated to v5.0.0 (beta 32): Many issues with this version.

ℹ️ v1.70.2 used xterm v4.20.0 (beta 20).

fritterhoff · 2022-09-04T12:11:14Z

So I can at least point to the commit where this change was introduced: microsoft/vscode@2f72682

Well... xterm has been updated to v5.0.0 (beta 32): Many issues with this version.

ℹ️ v1.70.2 used xterm v4.20.0 (beta 20).

I also saw the quite large amount of open issues but I'm not sure which actually are related to our problems. I only recognized that the upstream vscode project updated their dependency since the release of 1.71.

benz0li · 2022-09-04T13:09:46Z

for safari, we don't enable webgl https://github.com/microsoft/vscode/blob/aa93bc5e86dc929f07c5bfa51771c43664d15bdd/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts#L1589

Originally posted by @meganrogge in xtermjs/xterm.js#3357 (comment)

fritterhoff · 2022-09-05T05:04:37Z

Out of curiosity. What Safari version do you use? According to https://caniuse.com/webgl2 webgl should be supported by the latest Safari versions.

benz0li · 2022-09-05T05:30:27Z

Out of curiosity. What Safari version do you use?

Safari v15.6.1.

According to https://caniuse.com/webgl2 webgl should be supported by the latest Safari versions.

Yes it is.

Disabled by default in VS Code: https://github.com/microsoft/vscode/blob/784b0177c56c607789f9638da7b6bf3230d47a8c/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts#L285
ℹ️ Link to codebase of v1.71.0

fritterhoff · 2022-09-05T05:39:01Z

Ah... I missed the edit in your comment yesterday.

fritterhoff · 2022-09-05T07:29:20Z

So I can at least confirm that the rendering of the terminal has a bug on iPads too, even though it is not the same as mentioned by you @benz0li.

benz0li · 2022-09-05T08:45:47Z

[...], even though it is not the same as mentioned by you @benz0li.

It is the same issue. The Terminal looks like this after a reload:

benz0li · 2022-09-05T09:18:48Z

Cross reference: microsoft/vscode#160070

jsjoeio · 2022-09-06T16:47:12Z

Sorry, we had yesterday off for holiday in the US!

Can I just say this is such an awesome PR/conversation to come back to? Y'all are replacing me and @code-asher 😂

I have to tackle some other priorities but will get to this either today or tomorrow. And yes, I'm on macOS so I can test the Safari weirdness! Thank you two for getting the ball rolling!

codecov · 2022-09-06T17:02:00Z

Codecov Report

Merging #5535 (83deac8) into main (a1cf4b9) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5535   +/-   ##
=======================================
  Coverage   72.44%   72.44%           
=======================================
  Files          30       30           
  Lines        1673     1673           
  Branches      366      366           
=======================================
  Hits         1212     1212           
  Misses        398      398           
  Partials       63       63

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a1cf4b9...83deac8. Read the comment docs.

jsjoeio · 2022-09-06T21:12:03Z

Notes

Tested locally and things seem to be working 👍🏼

Safari

Terminal works if I turn off GPU acceleration.

jsjoeio

I think all of this looks fine. At least I don't see anything to be concerned about but I also have trouble reviewing these. I'll defer my review to @code-asher

jsjoeio · 2022-09-06T21:13:37Z

patches/proxy-uri.diff

+		const result: ResolverResult = { authority: { authority, host: host, port: port, connectionToken }, options };
+ 		RemoteAuthorities.set(authority, result.authority.host, result.authority.port);
+ 		this._cache.set(authority, result);
+ 		this._onDidChangeConnectionData.fire();


note to self: this change is fine, right?

jsjoeio · 2022-09-06T21:13:49Z

patches/telemetry.diff

@@ -1,5 +1,7 @@
 Add support for telemetry endpoint

+Contains some fixes included in https://github.com/microsoft/vscode/commit/b108bc8294ce920fcf2ee8d53f97c3bcf3316e1c


Nice addition 👍🏼

fritterhoff · 2022-09-08T03:02:48Z

Since upstream vs code "hot-fixed" the safari bug I would suggest to include this fix in code-server as well?

fritterhoff · 2022-09-08T04:40:25Z

@jsjoeio @benz0li I included the changes done by microsoft/vscode#160332

For simplicity I placed it in a single diff file. That can be removed in case of a new release/update.

jsjoeio · 2022-09-08T16:36:49Z

For simplicity I placed it in a single diff file. That can be removed in case of a new release/update.

You rock. Thanks for doing that! I'm still reviewing this but should be done today.

jsjoeio · 2022-09-08T18:11:56Z

Looking really good! I need to test the safari patch and the last three and then we should be good.

jsjoeio · 2022-09-08T19:51:17Z

@fritterhoff I was talking to @code-asher about the heartbeat patch. Looks like upstream implemented it here: microsoft/vscode#159239

Do you mind removing it in this PR? Otherwise I can do it in a followup PR.

jsjoeio · 2022-09-08T19:55:16Z

Safari fix works!

jsjoeio · 2022-09-08T22:15:40Z

I think the x86-64 job is timing out. You can increase this to 20m here

jsjoeio

I tested everything and it all looks good! @fritterhoff feel free to mark ready and then I can merge

jsjoeio · 2022-09-08T22:16:54Z

Oops, fat finger click on the ready for review

fritterhoff · 2022-09-09T03:50:21Z

I think the x86-64 job is timing out. You can increase this to 20m here

Done

@fritterhoff I was talking to @code-asher about the heartbeat patch. Looks like upstream implemented it here: microsoft/vscode#159239

Do you mind removing it in this PR? Otherwise I can do it in a followup PR.

Would do that in a new PR and remove it in the next release?

jsjoeio · 2022-09-09T16:10:05Z

Would do that in a new PR and remove it in the next release?

Oh! Good call. I thought it was in this version but looks like we need to wait for the next release.

jsjoeio

Woohoo!

jsjoeio · 2022-09-09T16:11:48Z

Opened an issue to track heartbeat removal: #5544

benz0li · 2022-09-09T17:20:57Z

code-server-4.6.1-1-linux-amd64.tar.gz of the artifacts is deployed at https://vscode-r.jupyter.b-data.ch; Image R (base:test) + code-server.

benz0li · 2022-09-09T18:17:05Z

@jsjoeio The footprint of code-server-4.6.1-1-linux-amd64 has increased to 626 MB. A plus of 264 MB compared to code-server-4.6.1-linux-amd64, whose footprint was 362 MB.

There is grpc in node_modules of code-server-4.6.1-1-linux-amd64 which alone is 236 MB in size.
👉 This package was not part of code-server-4.6.1-linux-amd64.

jsjoeio · 2022-09-09T18:53:17Z

That is a substantial increase. Thank you for pointing that out @benz0li! Let me and @code-asher dig in and see why that is.

code-asher · 2022-09-09T19:00:39Z

Hmm I am not sure; yarn why grpc does not seem to show anything.

jsjoeio · 2022-09-09T19:43:15Z

We figured it out using npm-why. Turns out @coder/logger -> @google-cloud/logging -> grpc

@google-cloud/logging is a peerDependencies and with our npm changes, it's being included in the build. We realized it's not being used so we're removing it from our logging library and bumping the version (coder/ts-logger#16). We'll then update in code-server before releasing 4.7.0

fritterhoff commented Sep 3, 2022

View reviewed changes

fritterhoff added 2 commits September 3, 2022 10:01

chore: update upstream code

9c40709

update patches for vs 1.71.0

9676fe6

the cli fix seems to be fixed in upstream, the telemtry patch requires (again) some fixing and adjustments.

benz0li mentioned this pull request Sep 6, 2022

Fix safari not properly rendering Terminal microsoft/vscode#160144

Closed

jsjoeio reviewed Sep 6, 2022

View reviewed changes

add safari fix.

a688920

Merge branch 'main' into main

ddbe271

jsjoeio approved these changes Sep 8, 2022

View reviewed changes

jsjoeio marked this pull request as ready for review September 8, 2022 22:16

jsjoeio requested a review from a team as a code owner September 8, 2022 22:16

jsjoeio marked this pull request as draft September 8, 2022 22:16

fritterhoff marked this pull request as ready for review September 9, 2022 03:40

increase ci timeout

83deac8

jsjoeio approved these changes Sep 9, 2022

View reviewed changes

jsjoeio enabled auto-merge (squash) September 9, 2022 16:10

jsjoeio mentioned this pull request Sep 9, 2022

[Chore]: Remove heartbeat patch #5544

Closed

jsjoeio merged commit b486354 into coder:main Sep 9, 2022

benz0li mentioned this pull request Jan 28, 2023

[Bug]: Terminal not showing up correct, and did not working at all after updating to 4.9.1 from 4.8.3 #5981

Closed

4 tasks

benz0li mentioned this pull request Feb 13, 2023

Update Code to 1.75.0 #6004

Merged

		@@ -1,5 +1,7 @@
		Add support for telemetry endpoint

		Contains some fixes included in https://github.com/microsoft/vscode/commit/b108bc8294ce920fcf2ee8d53f97c3bcf3316e1c

Update to 1.71 #5535

Update to 1.71 #5535

Conversation

fritterhoff commented Sep 3, 2022 • edited by jsjoeio Loading

Testing Plan

Patches tested

fritterhoff Sep 3, 2022

Choose a reason for hiding this comment

fritterhoff commented Sep 3, 2022

benz0li commented Sep 3, 2022

benz0li commented Sep 3, 2022 • edited Loading

fritterhoff commented Sep 3, 2022 • edited Loading

fritterhoff commented Sep 4, 2022

fritterhoff commented Sep 4, 2022

fritterhoff commented Sep 4, 2022 • edited Loading

benz0li commented Sep 4, 2022 • edited Loading

1.71.0

Screenshot

Log

1.70.2

Screenshot

Log

benz0li commented Sep 4, 2022

Settings

Settings (JSON)

fritterhoff commented Sep 4, 2022

fritterhoff commented Sep 4, 2022 • edited Loading

benz0li commented Sep 4, 2022 • edited Loading

Safari: Web Console

Safari: Web Network

benz0li commented Sep 4, 2022

benz0li commented Sep 4, 2022

fritterhoff commented Sep 4, 2022

benz0li commented Sep 4, 2022

fritterhoff commented Sep 5, 2022

benz0li commented Sep 5, 2022

fritterhoff commented Sep 5, 2022

fritterhoff commented Sep 5, 2022 • edited Loading

benz0li commented Sep 5, 2022

benz0li commented Sep 5, 2022

jsjoeio commented Sep 6, 2022

codecov bot commented Sep 6, 2022 • edited Loading

Codecov Report

jsjoeio commented Sep 6, 2022

Notes

Safari

jsjoeio left a comment

Choose a reason for hiding this comment

jsjoeio Sep 6, 2022

Choose a reason for hiding this comment

jsjoeio Sep 6, 2022

Choose a reason for hiding this comment

fritterhoff commented Sep 8, 2022

fritterhoff commented Sep 8, 2022 • edited Loading

jsjoeio commented Sep 8, 2022

jsjoeio commented Sep 8, 2022

jsjoeio commented Sep 8, 2022

jsjoeio commented Sep 8, 2022

jsjoeio commented Sep 8, 2022

jsjoeio left a comment

Choose a reason for hiding this comment

jsjoeio commented Sep 8, 2022

fritterhoff commented Sep 9, 2022

jsjoeio commented Sep 9, 2022

jsjoeio left a comment

Choose a reason for hiding this comment

jsjoeio commented Sep 9, 2022

benz0li commented Sep 9, 2022

benz0li commented Sep 9, 2022 • edited Loading

jsjoeio commented Sep 9, 2022

code-asher commented Sep 9, 2022

jsjoeio commented Sep 9, 2022

fritterhoff commented Sep 3, 2022 •

edited by jsjoeio

Loading

benz0li commented Sep 3, 2022 •

edited

Loading

fritterhoff commented Sep 3, 2022 •

edited

Loading

fritterhoff commented Sep 4, 2022 •

edited

Loading

benz0li commented Sep 4, 2022 •

edited

Loading

fritterhoff commented Sep 4, 2022 •

edited

Loading

benz0li commented Sep 4, 2022 •

edited

Loading

fritterhoff commented Sep 5, 2022 •

edited

Loading

codecov bot commented Sep 6, 2022 •

edited

Loading

fritterhoff commented Sep 8, 2022 •

edited

Loading

benz0li commented Sep 9, 2022 •

edited

Loading