diff options
author | Kristóf Marussy <kristof@marussy.com> | 2022-02-28 01:13:06 +0100 |
---|---|---|
committer | Kristóf Marussy <kristof@marussy.com> | 2022-03-06 18:56:49 +0100 |
commit | 2a5f7e3fecc98debea2b3408662d402a1e1681a0 (patch) | |
tree | 39b363cd8453007a8b6537e01813e0c8e61fe46c | |
parent | fix(service-preload): Browser view canvas background (diff) | |
download | sophie-2a5f7e3fecc98debea2b3408662d402a1e1681a0.tar.gz sophie-2a5f7e3fecc98debea2b3408662d402a1e1681a0.tar.zst sophie-2a5f7e3fecc98debea2b3408662d402a1e1681a0.zip |
feat: Handle service load failures
Adds a "failed" state for services where the BrowserView and WebContents
should be left around to keep history and allow people to navigate back.
Access to the browser history otherwise doesn't seem possible
(see https://github.com/electron/electron/issues/26727
and https://github.com/electron/electron/issues/7186),
so destroying BrowserView and managing our own history is not possible.
Also keep https://github.com/electron/electron/issues/24113 in mind.
Signed-off-by: Kristóf Marussy <kristof@marussy.com>
8 files changed, 121 insertions, 67 deletions
diff --git a/packages/main/src/infrastructure/electron/impl/ElectronServiceView.ts b/packages/main/src/infrastructure/electron/impl/ElectronServiceView.ts index e5fdf11..d90ff19 100644 --- a/packages/main/src/infrastructure/electron/impl/ElectronServiceView.ts +++ b/packages/main/src/infrastructure/electron/impl/ElectronServiceView.ts | |||
@@ -22,12 +22,15 @@ import type { BrowserViewBounds } from '@sophie/shared'; | |||
22 | import { BrowserView } from 'electron'; | 22 | import { BrowserView } from 'electron'; |
23 | 23 | ||
24 | import type Service from '../../../stores/Service'; | 24 | import type Service from '../../../stores/Service'; |
25 | import { getLogger } from '../../../utils/log'; | ||
25 | import type Resources from '../../resources/Resources'; | 26 | import type Resources from '../../resources/Resources'; |
26 | import type { ServiceView } from '../types'; | 27 | import type { ServiceView } from '../types'; |
27 | 28 | ||
28 | import ElectronPartition from './ElectronPartition'; | 29 | import ElectronPartition from './ElectronPartition'; |
29 | import type ElectronViewFactory from './ElectronViewFactory'; | 30 | import type ElectronViewFactory from './ElectronViewFactory'; |
30 | 31 | ||
32 | const log = getLogger('ElectronServiceView'); | ||
33 | |||
31 | export default class ElectronServiceView implements ServiceView { | 34 | export default class ElectronServiceView implements ServiceView { |
32 | readonly id: string; | 35 | readonly id: string; |
33 | 36 | ||
@@ -72,20 +75,39 @@ export default class ElectronServiceView implements ServiceView { | |||
72 | } | 75 | } |
73 | }); | 76 | }); |
74 | 77 | ||
78 | webContents.on( | ||
79 | 'did-fail-load', | ||
80 | (_event, errorCode, errorDesc, url, isMainFrame) => { | ||
81 | if (isMainFrame) { | ||
82 | setLocation(url); | ||
83 | service.setFailed(errorCode, errorDesc); | ||
84 | log.warn( | ||
85 | 'Failed to load', | ||
86 | url, | ||
87 | 'in service', | ||
88 | this.id, | ||
89 | errorCode, | ||
90 | errorDesc, | ||
91 | ); | ||
92 | } | ||
93 | }, | ||
94 | ); | ||
95 | |||
75 | webContents.on('page-title-updated', (_event, title) => { | 96 | webContents.on('page-title-updated', (_event, title) => { |
76 | service.setTitle(title); | 97 | service.setTitle(title); |
77 | }); | 98 | }); |
78 | 99 | ||
79 | webContents.on('did-start-loading', () => { | 100 | webContents.on('did-start-loading', () => { |
80 | service.startedLoading(); | 101 | service.startLoading(); |
81 | }); | 102 | }); |
82 | 103 | ||
83 | webContents.on('did-stop-loading', () => { | 104 | webContents.on('did-stop-loading', () => { |
84 | service.finishedLoading(); | 105 | service.finishLoading(); |
85 | }); | 106 | }); |
86 | 107 | ||
87 | webContents.on('render-process-gone', () => { | 108 | webContents.on('render-process-gone', (_event, details) => { |
88 | service.crashed(); | 109 | const { reason, exitCode } = details; |
110 | service.setCrashed(reason, exitCode); | ||
89 | }); | 111 | }); |
90 | } | 112 | } |
91 | 113 | ||
@@ -93,8 +115,10 @@ export default class ElectronServiceView implements ServiceView { | |||
93 | return this.browserView.webContents.id; | 115 | return this.browserView.webContents.id; |
94 | } | 116 | } |
95 | 117 | ||
96 | loadURL(url: string): Promise<void> { | 118 | loadURL(url: string): void { |
97 | return this.browserView.webContents.loadURL(url); | 119 | this.browserView.webContents.loadURL(url).catch((error) => { |
120 | log.warn('Error while loading', url, 'in service', this.id, error); | ||
121 | }); | ||
98 | } | 122 | } |
99 | 123 | ||
100 | goBack(): void { | 124 | goBack(): void { |
diff --git a/packages/main/src/infrastructure/electron/types.ts b/packages/main/src/infrastructure/electron/types.ts index 63974ce..7b04a6b 100644 --- a/packages/main/src/infrastructure/electron/types.ts +++ b/packages/main/src/infrastructure/electron/types.ts | |||
@@ -55,7 +55,7 @@ export interface ServiceView { | |||
55 | 55 | ||
56 | readonly partitionId: string; | 56 | readonly partitionId: string; |
57 | 57 | ||
58 | loadURL(url: string): Promise<void>; | 58 | loadURL(url: string): void; |
59 | 59 | ||
60 | goBack(): void; | 60 | goBack(): void; |
61 | 61 | ||
diff --git a/packages/main/src/reactions/loadServices.ts b/packages/main/src/reactions/loadServices.ts index 533ef95..4ef6131 100644 --- a/packages/main/src/reactions/loadServices.ts +++ b/packages/main/src/reactions/loadServices.ts | |||
@@ -78,36 +78,29 @@ export default function loadServices( | |||
78 | 78 | ||
79 | const viewsToDispose = new Map(servicesToViews); | 79 | const viewsToDispose = new Map(servicesToViews); |
80 | servicesById.forEach((service, serviceId) => { | 80 | servicesById.forEach((service, serviceId) => { |
81 | if (service.shouldBeLoaded) { | 81 | if (!service.shouldBeLoaded) { |
82 | let view = servicesToViews.get(serviceId); | 82 | return; |
83 | const { | 83 | } |
84 | settings: { | 84 | let view = servicesToViews.get(serviceId); |
85 | profile: { id: profileId }, | 85 | const { |
86 | }, | 86 | settings: { |
87 | } = service; | 87 | profile: { id: profileId }, |
88 | if (view === undefined || view.partitionId !== profileId) { | 88 | }, |
89 | log.debug('Creating view for service', serviceId); | 89 | } = service; |
90 | const partition = profilesToPartitions.get(profileId); | 90 | if (view === undefined || view.partitionId !== profileId) { |
91 | if (partition === undefined) { | 91 | log.debug('Creating view for service', serviceId); |
92 | throw new Error(`Missing Partition ${profileId}`); | 92 | const partition = profilesToPartitions.get(profileId); |
93 | } | 93 | if (partition === undefined) { |
94 | view = viewFactory.createServiceView(service, partition); | 94 | throw new Error(`Missing Partition ${profileId}`); |
95 | view.setBounds(store.browserViewBounds); | ||
96 | servicesToViews.set(serviceId, view); | ||
97 | service.setServiceView(view); | ||
98 | const { urlToLoad } = service; | ||
99 | view.loadURL(urlToLoad).catch((error) => { | ||
100 | log.warn( | ||
101 | 'Cannot URL', | ||
102 | urlToLoad, | ||
103 | 'for service', | ||
104 | serviceId, | ||
105 | error, | ||
106 | ); | ||
107 | }); | ||
108 | } else { | ||
109 | viewsToDispose.delete(serviceId); | ||
110 | } | 95 | } |
96 | view = viewFactory.createServiceView(service, partition); | ||
97 | view.setBounds(store.browserViewBounds); | ||
98 | servicesToViews.set(serviceId, view); | ||
99 | service.setServiceView(view); | ||
100 | const { urlToLoad } = service; | ||
101 | view.loadURL(urlToLoad); | ||
102 | } else { | ||
103 | viewsToDispose.delete(serviceId); | ||
111 | } | 104 | } |
112 | }); | 105 | }); |
113 | 106 | ||
diff --git a/packages/main/src/stores/MainStore.ts b/packages/main/src/stores/MainStore.ts index 21d7a63..dedc740 100644 --- a/packages/main/src/stores/MainStore.ts +++ b/packages/main/src/stores/MainStore.ts | |||
@@ -56,7 +56,7 @@ const MainStore = types | |||
56 | }, | 56 | }, |
57 | get visibleService(): Service | undefined { | 57 | get visibleService(): Service | undefined { |
58 | const { selectedService } = this.settings; | 58 | const { selectedService } = this.settings; |
59 | return selectedService !== undefined && selectedService.shouldBeLoaded | 59 | return selectedService !== undefined && selectedService.shouldBeVisible |
60 | ? selectedService | 60 | ? selectedService |
61 | : undefined; | 61 | : undefined; |
62 | }, | 62 | }, |
diff --git a/packages/main/src/stores/Service.ts b/packages/main/src/stores/Service.ts index cbd8662..d98e52e 100644 --- a/packages/main/src/stores/Service.ts +++ b/packages/main/src/stores/Service.ts | |||
@@ -40,7 +40,10 @@ const Service = defineServiceModel(ServiceSettings) | |||
40 | return self.currentUrl ?? self.settings.url; | 40 | return self.currentUrl ?? self.settings.url; |
41 | }, | 41 | }, |
42 | get shouldBeLoaded(): boolean { | 42 | get shouldBeLoaded(): boolean { |
43 | return self.state !== 'crashed'; | 43 | return !self.crashed; |
44 | }, | ||
45 | get shouldBeVisible(): boolean { | ||
46 | return this.shouldBeLoaded && !self.failed; | ||
44 | }, | 47 | }, |
45 | })) | 48 | })) |
46 | .volatile( | 49 | .volatile( |
@@ -67,17 +70,20 @@ const Service = defineServiceModel(ServiceSettings) | |||
67 | setTitle(title: string): void { | 70 | setTitle(title: string): void { |
68 | self.title = title; | 71 | self.title = title; |
69 | }, | 72 | }, |
70 | startedLoading(): void { | 73 | startLoading(): void { |
71 | self.state = 'loading'; | 74 | self.state = { type: 'loading' }; |
72 | }, | 75 | }, |
73 | finishedLoading(): void { | 76 | finishLoading(): void { |
74 | if (self.state === 'loading') { | 77 | if (self.loading) { |
75 | // Do not overwrite crashed state if the service haven't been reloaded yet. | 78 | // Do not overwrite crashed state if the service haven't been reloaded yet. |
76 | self.state = 'loaded'; | 79 | self.state = { type: 'loaded' }; |
77 | } | 80 | } |
78 | }, | 81 | }, |
79 | crashed(): void { | 82 | setFailed(errorCode: number, errorDesc: string): void { |
80 | self.state = 'crashed'; | 83 | self.state = { type: 'failed', errorCode, errorDesc }; |
84 | }, | ||
85 | setCrashed(reason: string, exitCode: number): void { | ||
86 | self.state = { type: 'crashed', reason, exitCode }; | ||
81 | }, | 87 | }, |
82 | setUnreadCount({ direct, indirect }: UnreadCount): void { | 88 | setUnreadCount({ direct, indirect }: UnreadCount): void { |
83 | if (direct !== undefined) { | 89 | if (direct !== undefined) { |
@@ -98,7 +104,7 @@ const Service = defineServiceModel(ServiceSettings) | |||
98 | }, | 104 | }, |
99 | reload(ignoreCache = false): void { | 105 | reload(ignoreCache = false): void { |
100 | if (self.serviceView === undefined) { | 106 | if (self.serviceView === undefined) { |
101 | this.startedLoading(); | 107 | self.state = { type: 'initializing' }; |
102 | } else { | 108 | } else { |
103 | self.serviceView?.reload(ignoreCache); | 109 | self.serviceView?.reload(ignoreCache); |
104 | } | 110 | } |
@@ -109,12 +115,9 @@ const Service = defineServiceModel(ServiceSettings) | |||
109 | go(url: string): void { | 115 | go(url: string): void { |
110 | if (self.serviceView === undefined) { | 116 | if (self.serviceView === undefined) { |
111 | self.currentUrl = url; | 117 | self.currentUrl = url; |
112 | this.startedLoading(); | 118 | self.state = { type: 'initializing' }; |
113 | } else { | 119 | } else { |
114 | self.serviceView?.loadURL(url).catch((error) => { | 120 | self.serviceView?.loadURL(url); |
115 | log.warn('Error while loading', url, error); | ||
116 | this.crashed(); | ||
117 | }); | ||
118 | } | 121 | } |
119 | }, | 122 | }, |
120 | goHome(): void { | 123 | goHome(): void { |
diff --git a/packages/renderer/src/components/locationBar/NavigationButtons.tsx b/packages/renderer/src/components/locationBar/NavigationButtons.tsx index e71d3d8..5c5c959 100644 --- a/packages/renderer/src/components/locationBar/NavigationButtons.tsx +++ b/packages/renderer/src/components/locationBar/NavigationButtons.tsx | |||
@@ -59,7 +59,7 @@ function NavigationButtons({ | |||
59 | > | 59 | > |
60 | {direction === 'ltr' ? <IconArrowForward /> : <IconArrowBack />} | 60 | {direction === 'ltr' ? <IconArrowForward /> : <IconArrowBack />} |
61 | </IconButton> | 61 | </IconButton> |
62 | {service?.state === 'loading' ? ( | 62 | {service?.loading ?? false ? ( |
63 | <IconButton aria-label="Stop" onClick={() => service?.stop()}> | 63 | <IconButton aria-label="Stop" onClick={() => service?.stop()}> |
64 | <IconStop /> | 64 | <IconStop /> |
65 | </IconButton> | 65 | </IconButton> |
diff --git a/packages/renderer/src/components/sidebar/ToggleLocationBarButton.tsx b/packages/renderer/src/components/sidebar/ToggleLocationBarButton.tsx index d2f0745..57b17e9 100644 --- a/packages/renderer/src/components/sidebar/ToggleLocationBarButton.tsx +++ b/packages/renderer/src/components/sidebar/ToggleLocationBarButton.tsx | |||
@@ -56,7 +56,7 @@ function ToggleLocationBarButton(): JSX.Element { | |||
56 | onClick={() => settings.toggleLocationBar()} | 56 | onClick={() => settings.toggleLocationBar()} |
57 | > | 57 | > |
58 | <ToggleLocationBarIcon | 58 | <ToggleLocationBarIcon |
59 | loading={selectedService?.state === 'loading'} | 59 | loading={selectedService?.loading ?? false} |
60 | show={showLocationBar} | 60 | show={showLocationBar} |
61 | /> | 61 | /> |
62 | </IconButton> | 62 | </IconButton> |
diff --git a/packages/shared/src/stores/ServiceBase.ts b/packages/shared/src/stores/ServiceBase.ts index cde403b..4a17bc5 100644 --- a/packages/shared/src/stores/ServiceBase.ts +++ b/packages/shared/src/stores/ServiceBase.ts | |||
@@ -23,20 +23,54 @@ import { IAnyModelType, Instance, types } from 'mobx-state-tree'; | |||
23 | import ServiceSettingsBase from './ServiceSettingsBase'; | 23 | import ServiceSettingsBase from './ServiceSettingsBase'; |
24 | 24 | ||
25 | export function defineServiceModel<TS extends IAnyModelType>(settings: TS) { | 25 | export function defineServiceModel<TS extends IAnyModelType>(settings: TS) { |
26 | return types.model('Service', { | 26 | return types |
27 | id: types.identifier, | 27 | .model('Service', { |
28 | settings, | 28 | id: types.identifier, |
29 | currentUrl: types.maybe(types.string), | 29 | settings, |
30 | canGoBack: false, | 30 | currentUrl: types.maybe(types.string), |
31 | canGoForward: false, | 31 | canGoBack: false, |
32 | title: types.maybe(types.string), | 32 | canGoForward: false, |
33 | state: types.optional( | 33 | title: types.maybe(types.string), |
34 | types.enumeration('ServiceState', ['loading', 'loaded', 'crashed']), | 34 | state: types.optional( |
35 | 'loading', | 35 | types.union( |
36 | ), | 36 | types.model({ |
37 | directMessageCount: 0, | 37 | type: types.literal('initializing'), |
38 | indirectMessageCount: 0, | 38 | }), |
39 | }); | 39 | types.model({ |
40 | type: types.literal('loading'), | ||
41 | }), | ||
42 | types.model({ | ||
43 | type: types.literal('loaded'), | ||
44 | }), | ||
45 | types.model({ | ||
46 | type: types.literal('failed'), | ||
47 | errorCode: types.integer, | ||
48 | errorDesc: types.string, | ||
49 | }), | ||
50 | types.model({ | ||
51 | type: types.literal('crashed'), | ||
52 | reason: types.string, | ||
53 | exitCode: types.integer, | ||
54 | }), | ||
55 | ), | ||
56 | { type: 'initializing' }, | ||
57 | ), | ||
58 | directMessageCount: 0, | ||
59 | indirectMessageCount: 0, | ||
60 | }) | ||
61 | .views((self) => ({ | ||
62 | get loading(): boolean { | ||
63 | return ( | ||
64 | self.state.type === 'initializing' || self.state.type === 'loading' | ||
65 | ); | ||
66 | }, | ||
67 | get failed(): boolean { | ||
68 | return self.state.type === 'failed'; | ||
69 | }, | ||
70 | get crashed(): boolean { | ||
71 | return self.state.type === 'crashed'; | ||
72 | }, | ||
73 | })); | ||
40 | } | 74 | } |
41 | 75 | ||
42 | const ServiceBase = /* @__PURE__ */ (() => | 76 | const ServiceBase = /* @__PURE__ */ (() => |