Skip to content

Commit e46dbec

Browse files
zeripathsilverwindtechknowlogick
authored
Move EventSource to SharedWorker (#12095) (#12130)
* Move EventSource to SharedWorker (#12095) Backport #12095 Move EventSource to use a SharedWorker. This prevents issues with HTTP/1.1 open browser connections from preventing gitea from opening multiple tabs. Also allow setting EVENT_SOURCE_UPDATE_TIME to disable EventSource updating Fix #11978 Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: silverwind <me@silverwind.io> Co-authored-by: techknowlogick <techknowlogick@gitea.io> * Bugfix for shared event source For some reason our eslint configuration is not working correctly and a bug has become apparent when trying to backport this to 1.12. Signed-off-by: Andrew Thornton <art27@cantab.net> * Re-fix #12095 again Unfortunately some of the suggested changes to #12095 introduced bugs which due to caching behaviour of sharedworkers were not caught on simple tests. These are as follows: * Changing from simple for loop to use includes here: ```js register(port) { if (!this.clients.includes(port)) return; this.clients.push(port); port.postMessage({ type: 'status', message: `registered to ${this.url}`, }); } ``` The additional `!` prevents any clients from being added and should read: ```js if (this.clients.includes(port)) return; ``` * Dropping the use of jQuery `$(...)` selection and using DOM `querySelector` here: ```js async function receiveUpdateCount(event) { try { const data = JSON.parse(event.data); const notificationCount = document.querySelector('.notification_count'); if (data.Count > 0) { notificationCount.classList.remove('hidden'); } else { notificationCount.classList.add('hidden'); } notificationCount.text() = `${data.Count}`; await updateNotificationTable(); } catch (error) { console.error(error, event); } } ``` Requires that `notificationCount.text()` be changed to use `textContent` instead. Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: silverwind <me@silverwind.io> Co-authored-by: techknowlogick <techknowlogick@gitea.io>
1 parent 8f64017 commit e46dbec

File tree

9 files changed

+211
-37
lines changed

9 files changed

+211
-37
lines changed

‎.eslintrc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ globals:
2525
Tribute: false
2626

2727
overrides:
28-
- files: ["web_src/**/*.worker.js", "web_src/js/serviceworker.js"]
28+
- files: ["web_src/**/*worker.js"]
2929
env:
3030
worker: true
3131
rules:

‎custom/conf/app.ini.sample‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ MIN_TIMEOUT = 10s
211211
MAX_TIMEOUT = 60s
212212
TIMEOUT_STEP = 10s
213213
; This setting determines how often the db is queried to get the latest notification counts.
214-
; If the browser client supports EventSource, it will be used in preference to polling notification.
214+
; If the browser client supports EventSource and SharedWorker, a SharedWorker will be used in preference to polling notification. Set to -1 to disable the EventSource
215215
EVENT_SOURCE_UPDATE_TIME = 10s
216216

217217
[markdown]

‎docs/content/doc/advanced/config-cheat-sheet.en-us.md‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,7 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
148148
- `MIN_TIMEOUT`: **10s**: These options control how often notification endpoint is polled to update the notification count. On page load the notification count will be checked after `MIN_TIMEOUT`. The timeout will increase to `MAX_TIMEOUT` by `TIMEOUT_STEP` if the notification count is unchanged. Set MIN_TIMEOUT to 0 to turn off.
149149
- `MAX_TIMEOUT`: **60s**.
150150
- `TIMEOUT_STEP`: **10s**.
151-
- `EVENT_SOURCE_UPDATE_TIME`: **10s**: This setting determines how often the database is queried to update notification counts. If the browser client supports `EventSource`, it will be used in preference to polling notification endpoint.
152-
151+
- `EVENT_SOURCE_UPDATE_TIME`: **10s**: This setting determines how often the database is queried to update notification counts. If the browser client supports `EventSource` and `SharedWorker`, a `SharedWorker` will be used in preference to polling notification endpoint. Set to **-1** to disable the `EventSource`.
153152

154153
## Markdown (`markdown`)
155154

‎modules/eventsource/manager_run.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ import (
1717

1818
// Init starts this eventsource
1919
func (m *Manager) Init() {
20+
if setting.UI.Notification.EventSourceUpdateTime <= 0 {
21+
return
22+
}
2023
go graceful.GetManager().RunWithShutdownContext(m.Run)
2124
}
2225

‎modules/templates/helper.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,8 @@ func NewFuncMap() []template.FuncMap {
289289
return ""
290290
}
291291
},
292-
"NotificationSettings": func() map[string]int {
293-
return map[string]int{
292+
"NotificationSettings": func() map[string]interface{} {
293+
return map[string]interface{}{
294294
"MinTimeout": int(setting.UI.Notification.MinTimeout / time.Millisecond),
295295
"TimeoutStep": int(setting.UI.Notification.TimeoutStep / time.Millisecond),
296296
"MaxTimeout": int(setting.UI.Notification.MaxTimeout / time.Millisecond),

‎web_src/js/features/eventsource.sharedworker.js‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
self.name = 'eventsource.sharedworker.js';
2+
3+
const sourcesByUrl = {};
4+
const sourcesByPort = {};
5+
6+
class Source {
7+
constructor(url) {
8+
this.url = url;
9+
this.eventSource = new EventSource(url);
10+
this.listening = {};
11+
this.clients = [];
12+
this.listen('open');
13+
this.listen('logout');
14+
this.listen('notification-count');
15+
this.listen('error');
16+
}
17+
18+
register(port) {
19+
if (this.clients.includes(port)) return;
20+
21+
this.clients.push(port);
22+
23+
port.postMessage({
24+
type: 'status',
25+
message: `registered to ${this.url}`,
26+
});
27+
}
28+
29+
deregister(port) {
30+
const portIdx = this.clients.indexOf(port);
31+
if (portIdx < 0) {
32+
return this.clients.length;
33+
}
34+
this.clients.splice(portIdx, 1);
35+
return this.clients.length;
36+
}
37+
38+
close() {
39+
if (!this.eventSource) return;
40+
41+
this.eventSource.close();
42+
this.eventSource = null;
43+
}
44+
45+
listen(eventType) {
46+
if (this.listening[eventType]) return;
47+
this.listening[eventType] = true;
48+
const self = this;
49+
this.eventSource.addEventListener(eventType, (event) => {
50+
self.notifyClients({
51+
type: eventType,
52+
data: event.data
53+
});
54+
});
55+
}
56+
57+
notifyClients(event) {
58+
for (const client of this.clients) {
59+
client.postMessage(event);
60+
}
61+
}
62+
63+
status(port) {
64+
port.postMessage({
65+
type: 'status',
66+
message: `url: ${this.url} readyState: ${this.eventSource.readyState}`,
67+
});
68+
}
69+
}
70+
71+
self.onconnect = (e) => {
72+
for (const port of e.ports) {
73+
port.addEventListener('message', (event) => {
74+
if (event.data.type === 'start') {
75+
const url = event.data.url;
76+
if (sourcesByUrl[url]) {
77+
// we have a Source registered to this url
78+
const source = sourcesByUrl[url];
79+
source.register(port);
80+
sourcesByPort[port] = source;
81+
return;
82+
}
83+
let source = sourcesByPort[port];
84+
if (source) {
85+
if (source.eventSource && source.url === url) return;
86+
87+
// How this has happened I don't understand...
88+
// deregister from that source
89+
const count = source.deregister(port);
90+
// Clean-up
91+
if (count === 0) {
92+
source.close();
93+
sourcesByUrl[source.url] = null;
94+
}
95+
}
96+
// Create a new Source
97+
source = new Source(url);
98+
source.register(port);
99+
sourcesByUrl[url] = source;
100+
sourcesByPort[port] = source;
101+
} else if (event.data.type === 'listen') {
102+
const source = sourcesByPort[port];
103+
source.listen(event.data.eventType);
104+
} else if (event.data.type === 'close') {
105+
const source = sourcesByPort[port];
106+
107+
if (!source) return;
108+
109+
const count = source.deregister(port);
110+
if (count === 0) {
111+
source.close();
112+
sourcesByUrl[source.url] = null;
113+
sourcesByPort[port] = null;
114+
}
115+
} else if (event.data.type === 'status') {
116+
const source = sourcesByPort[port];
117+
if (!source) {
118+
port.postMessage({
119+
type: 'status',
120+
message: 'not connected',
121+
});
122+
return;
123+
}
124+
source.status(port);
125+
} else {
126+
// just send it back
127+
port.postMessage({
128+
type: 'error',
129+
message: `received but don't know how to handle: ${event.data}`,
130+
});
131+
}
132+
});
133+
port.start();
134+
}
135+
};

‎web_src/js/features/notification.js‎

Lines changed: 64 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,44 +18,78 @@ export function initNotificationsTable() {
1818
});
1919
}
2020

21-
export function initNotificationCount() {
21+
async function receiveUpdateCount(event) {
22+
try {
23+
const data = JSON.parse(event.data);
24+
25+
const notificationCount = document.querySelector('.notification_count');
26+
if (data.Count > 0) {
27+
notificationCount.classList.remove('hidden');
28+
} else {
29+
notificationCount.classList.add('hidden');
30+
}
31+
32+
notificationCount.textContent = `${data.Count}`;
33+
await updateNotificationTable();
34+
} catch (error) {
35+
console.error(error, event);
36+
}
37+
}
38+
39+
export async function initNotificationCount() {
2240
const notificationCount = $('.notification_count');
2341

2442
if (!notificationCount.length) {
2543
return;
2644
}
2745

2846
if (NotificationSettings.EventSourceUpdateTime > 0 && !!window.EventSource) {
29-
// Try to connect to the event source first
30-
const source = new EventSource(`${AppSubUrl}/user/events`);
31-
source.addEventListener('notification-count', async (e) => {
32-
try {
33-
const data = JSON.parse(e.data);
34-
35-
const notificationCount = $('.notification_count');
36-
if (data.Count === 0) {
37-
notificationCount.addClass('hidden');
38-
} else {
39-
notificationCount.removeClass('hidden');
47+
// Try to connect to the event source via the shared worker first
48+
if (window.SharedWorker) {
49+
const worker = new SharedWorker(`${__webpack_public_path__}js/eventsource.sharedworker.js`, 'notification-worker');
50+
worker.addEventListener('error', (event) => {
51+
console.error(event);
52+
});
53+
worker.port.onmessageerror = () => {
54+
console.error('Unable to deserialize message');
55+
};
56+
worker.port.postMessage({
57+
type: 'start',
58+
url: `${window.location.origin}${AppSubUrl}/user/events`,
59+
});
60+
worker.port.addEventListener('message', (event) => {
61+
if (!event.data || !event.data.type) {
62+
console.error(event);
63+
return;
4064
}
41-
42-
notificationCount.text(`${data.Count}`);
43-
await updateNotificationTable();
44-
} catch (error) {
45-
console.error(error);
46-
}
47-
});
48-
source.addEventListener('logout', async (e) => {
49-
if (e.data !== 'here') {
50-
return;
51-
}
52-
source.close();
53-
window.location.href = AppSubUrl;
54-
});
55-
window.addEventListener('beforeunload', () => {
56-
source.close();
57-
});
58-
return;
65+
if (event.data.type === 'notification-count') {
66+
receiveUpdateCount(event.data);
67+
} else if (event.data.type === 'error') {
68+
console.error(event.data);
69+
} else if (event.data.type === 'logout') {
70+
if (event.data !== 'here') {
71+
return;
72+
}
73+
worker.port.postMessage({
74+
type: 'close',
75+
});
76+
worker.port.close();
77+
window.location.href = AppSubUrl;
78+
}
79+
});
80+
worker.port.addEventListener('error', (e) => {
81+
console.error(e);
82+
});
83+
worker.port.start();
84+
window.addEventListener('beforeunload', () => {
85+
worker.port.postMessage({
86+
type: 'close',
87+
});
88+
worker.port.close();
89+
});
90+
91+
return;
92+
}
5993
}
6094

6195
if (NotificationSettings.MinTimeout <= 0) {

‎web_src/js/index.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2455,7 +2455,6 @@ $(document).ready(async () => {
24552455
initTemplateSearch();
24562456
initContextPopups();
24572457
initNotificationsTable();
2458-
initNotificationCount();
24592458
initTribute();
24602459

24612460
// Repo clone url.
@@ -2502,6 +2501,7 @@ $(document).ready(async () => {
25022501
initClipboard(),
25032502
initUserHeatmap(),
25042503
initServiceWorker(),
2504+
initNotificationCount(),
25052505
]);
25062506
});
25072507

‎webpack.config.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ module.exports = {
3838
serviceworker: [
3939
resolve(__dirname, 'web_src/js/serviceworker.js'),
4040
],
41+
'eventsource.sharedworker': [
42+
resolve(__dirname, 'web_src/js/features/eventsource.sharedworker.js'),
43+
],
4144
icons: glob('node_modules/@primer/octicons/build/svg/**/*.svg'),
4245
...themes,
4346
},

0 commit comments

Comments
 (0)