From 98b51b666d04971d9d30c1ea708ead69d4294215 Mon Sep 17 00:00:00 2001 From: Sag Date: Tue, 4 Jun 2024 16:20:49 +0200 Subject: [PATCH] Fixed ember concurrency cancellation errors (#20324) fixes https://linear.app/tryghost/issue/SLO-121 fixes https://linear.app/tryghost/issue/SLO-138 fixes https://linear.app/tryghost/issue/SLO-139 fixes https://linear.app/tryghost/issue/SLO-140 fixes https://linear.app/tryghost/issue/SLO-141 fixes https://linear.app/tryghost/issue/SLO-142 - ember-concurrency prevents two executions of the same task from running at the same time - when a task is cancelled, the library raises an error by default - however, we don't need to surface that error as the cancellation of the second execution is intentional --- .../app/components/gh-member-settings-form.js | 15 ++++++++-- .../app/components/koenig-lexical-editor.js | 24 ++++++++++++++-- ghost/admin/app/components/members/filter.js | 18 +++++++++--- .../admin/app/components/modal-member-tier.js | 13 +++++++-- ghost/admin/app/components/posts/analytics.js | 24 ++++++++++++---- .../app/components/posts/old-analytics.js | 28 +++++++++++++------ ghost/admin/app/controllers/members.js | 18 +++++++++--- ghost/admin/app/controllers/offer.js | 13 +++++++-- ghost/admin/app/routes/members.js | 13 ++++++++- 9 files changed, 134 insertions(+), 32 deletions(-) diff --git a/ghost/admin/app/components/gh-member-settings-form.js b/ghost/admin/app/components/gh-member-settings-form.js index 8596af3b45..d7f4162dec 100644 --- a/ghost/admin/app/components/gh-member-settings-form.js +++ b/ghost/admin/app/components/gh-member-settings-form.js @@ -1,9 +1,9 @@ import Component from '@glimmer/component'; import moment from 'moment-timezone'; import {action} from '@ember/object'; +import {didCancel, task} from 'ember-concurrency'; import {getNonDecimal, getSymbol} from 'ghost-admin/utils/currency'; import {inject as service} from '@ember/service'; -import {task} from 'ember-concurrency'; import {tracked} from '@glimmer/tracking'; export default class extends Component { @@ -137,8 +137,17 @@ export default class extends Component { @action setup() { - this.fetchTiers.perform(); - this.fetchNewsletters.perform(); + try { + this.fetchTiers.perform(); + this.fetchNewsletters.perform(); + } catch (e) { + // Do not throw cancellation errors + if (didCancel(e)) { + return; + } + + throw e; + } } @action diff --git a/ghost/admin/app/components/koenig-lexical-editor.js b/ghost/admin/app/components/koenig-lexical-editor.js index 5b6f111cb3..3112af1abf 100644 --- a/ghost/admin/app/components/koenig-lexical-editor.js +++ b/ghost/admin/app/components/koenig-lexical-editor.js @@ -273,7 +273,17 @@ export default class KoenigLexicalEditor extends Component { }; const fetchAutocompleteLinks = async () => { - const offers = await this.fetchOffersTask.perform(); + let offers = []; + try { + offers = await this.fetchOffersTask.perform(); + } catch (e) { + // Do not throw cancellation errors + if (didCancel(e)) { + return; + } + + throw e; + } const defaults = [ {label: 'Homepage', value: window.location.origin + '/'}, @@ -330,7 +340,17 @@ export default class KoenigLexicalEditor extends Component { }; const fetchLabels = async () => { - const labels = await this.fetchLabelsTask.perform(); + let labels = []; + try { + labels = await this.fetchLabelsTask.perform(); + } catch (e) { + // Do not throw cancellation errors + if (didCancel(e)) { + return; + } + + throw e; + } return labels.map(label => label.name); }; diff --git a/ghost/admin/app/components/members/filter.js b/ghost/admin/app/components/members/filter.js index 475ddf53eb..6993d1c4e7 100644 --- a/ghost/admin/app/components/members/filter.js +++ b/ghost/admin/app/components/members/filter.js @@ -4,8 +4,8 @@ import nql from '@tryghost/nql-lang'; import {AUDIENCE_FEEDBACK_FILTER, CREATED_AT_FILTER, EMAIL_CLICKED_FILTER, EMAIL_COUNT_FILTER, EMAIL_FILTER, EMAIL_OPENED_COUNT_FILTER, EMAIL_OPENED_FILTER, EMAIL_OPEN_RATE_FILTER, EMAIL_SENT_FILTER, LABEL_FILTER, LAST_SEEN_FILTER, NAME_FILTER, NEWSLETTERS_FILTERS, NEXT_BILLING_DATE_FILTER, OFFERS_FILTER, PLAN_INTERVAL_FILTER, SIGNUP_ATTRIBUTION_FILTER, STATUS_FILTER, SUBSCRIBED_FILTER, SUBSCRIPTION_ATTRIBUTION_FILTER, SUBSCRIPTION_START_DATE_FILTER, SUBSCRIPTION_STATUS_FILTER, TIER_FILTER} from './filters'; import {TrackedArray} from 'tracked-built-ins'; import {action} from '@ember/object'; +import {didCancel, task} from 'ember-concurrency'; import {inject as service} from '@ember/service'; -import {task} from 'ember-concurrency'; import {tracked} from '@glimmer/tracking'; function escapeNqlString(value) { @@ -234,9 +234,19 @@ export default class MembersFilter extends Component { async parseDefaultFilters() { // we need to make sure all the filters are loaded before parsing the default filter // otherwise the filter will be parsed with the wrong properties - await this.fetchTiers.perform(); - await this.fetchNewsletters.perform(); - await this.fetchOffers.perform(); + try { + await this.fetchTiers.perform(); + await this.fetchNewsletters.perform(); + await this.fetchOffers.perform(); + } catch (e) { + // Do not throw cancellation errors + if (didCancel(e)) { + return; + } + + throw e; + } + if (this.args.defaultFilterParam) { // check if it is different before parsing const validFilters = this.validFilters; diff --git a/ghost/admin/app/components/modal-member-tier.js b/ghost/admin/app/components/modal-member-tier.js index bc94dc2d1e..733d08e3e6 100644 --- a/ghost/admin/app/components/modal-member-tier.js +++ b/ghost/admin/app/components/modal-member-tier.js @@ -1,8 +1,8 @@ import ModalComponent from 'ghost-admin/components/modal-base'; import moment from 'moment-timezone'; import {action} from '@ember/object'; +import {didCancel, task} from 'ember-concurrency'; import {inject as service} from '@ember/service'; -import {task} from 'ember-concurrency'; import {tracked} from '@glimmer/tracking'; export default class ModalMemberTier extends ModalComponent { @@ -77,7 +77,16 @@ export default class ModalMemberTier extends ModalComponent { @action setup() { this.loadingTiers = true; - this.fetchTiers.perform(); + try { + this.fetchTiers.perform(); + } catch (e) { + // Do not throw cancellation errors + if (didCancel(e)) { + return; + } + + throw e; + } } @action diff --git a/ghost/admin/app/components/posts/analytics.js b/ghost/admin/app/components/posts/analytics.js index 3ec21fff26..0f81f7454d 100644 --- a/ghost/admin/app/components/posts/analytics.js +++ b/ghost/admin/app/components/posts/analytics.js @@ -201,18 +201,30 @@ export default class Analytics extends Component { } return this._fetchReferrersStats.perform(); } catch (e) { - if (!didCancel(e)) { - // re-throw the non-cancelation error - throw e; + // Do not throw cancellation errors + if (didCancel(e)) { + return; } + + throw e; } } async fetchLinks() { - if (this._fetchLinks.isRunning) { - return this._fetchLinks.last; + try { + if (this._fetchLinks.isRunning) { + return this._fetchLinks.last; + } + + return this._fetchLinks.perform(); + } catch (e) { + // Do not throw cancellation errors + if (didCancel(e)) { + return; + } + + throw e; } - return this._fetchLinks.perform(); } @task diff --git a/ghost/admin/app/components/posts/old-analytics.js b/ghost/admin/app/components/posts/old-analytics.js index dbc66cf564..a57bfb3706 100644 --- a/ghost/admin/app/components/posts/old-analytics.js +++ b/ghost/admin/app/components/posts/old-analytics.js @@ -171,18 +171,30 @@ export default class Analytics extends Component { } return this._fetchReferrersStats.perform(); } catch (e) { - if (!didCancel(e)) { - // re-throw the non-cancelation error - throw e; + // Do not throw cancellation errors + if (didCancel(e)) { + return; } + + throw e; } } async fetchLinks() { - if (this._fetchLinks.isRunning) { - return this._fetchLinks.last; + try { + if (this._fetchLinks.isRunning) { + return this._fetchLinks.last; + } + + return this._fetchLinks.perform(); + } catch (e) { + // Do not throw cancellation errors + if (didCancel(e)) { + return; + } + + throw e; } - return this._fetchLinks.perform(); } @task @@ -226,7 +238,7 @@ export default class Analytics extends Component { }, 2000); } - @task + @task({drop: true}) *_fetchReferrersStats() { let statsUrl = this.ghostPaths.url.api(`stats/referrers/posts/${this.post.id}`); let result = yield this.ajax.request(statsUrl); @@ -239,7 +251,7 @@ export default class Analytics extends Component { }); } - @task + @task({drop: true}) *_fetchLinks() { const filter = `post_id:'${this.post.id}'`; let statsUrl = this.ghostPaths.url.api(`links/`) + `?filter=${encodeURIComponent(filter)}`; diff --git a/ghost/admin/app/controllers/members.js b/ghost/admin/app/controllers/members.js index 45598c6d5c..016987fda2 100644 --- a/ghost/admin/app/controllers/members.js +++ b/ghost/admin/app/controllers/members.js @@ -7,11 +7,11 @@ import ghostPaths from 'ghost-admin/utils/ghost-paths'; import moment from 'moment-timezone'; import {A} from '@ember/array'; import {action} from '@ember/object'; +import {didCancel, task, timeout} from 'ember-concurrency'; import {ghPluralize} from 'ghost-admin/helpers/gh-pluralize'; import {inject} from 'ghost-admin/decorators/inject'; import {resetQueryParams} from 'ghost-admin/helpers/reset-query-params'; import {inject as service} from '@ember/service'; -import {task, timeout} from 'ember-concurrency'; import {tracked} from '@glimmer/tracking'; const PAID_PARAMS = [{ @@ -258,8 +258,18 @@ export default class MembersController extends Controller { @action refreshData() { - this.fetchMembersTask.perform(); - this.fetchLabelsTask.perform(); + try { + this.fetchMembersTask.perform(); + this.fetchLabelsTask.perform(); + } catch (e) { + // Do not throw cancellation errors + if (didCancel(e)) { + return; + } + + throw e; + } + this.membersStats.invalidate(); this.membersStats.fetchCounts(); this.membersStats.fetchMemberCount(); @@ -415,7 +425,7 @@ export default class MembersController extends Controller { this.searchParam = query; } - @task + @task({restartable: true}) *fetchLabelsTask() { yield this.store.query('label', {limit: 'all'}); } diff --git a/ghost/admin/app/controllers/offer.js b/ghost/admin/app/controllers/offer.js index 3fe7e73c22..5949efe9f8 100644 --- a/ghost/admin/app/controllers/offer.js +++ b/ghost/admin/app/controllers/offer.js @@ -4,11 +4,11 @@ import UnarchiveOfferModal from '../components/modals/offers/unarchive'; import config from 'ghost-admin/config/environment'; import copyTextToClipboard from 'ghost-admin/utils/copy-text-to-clipboard'; import {action} from '@ember/object'; +import {didCancel, task} from 'ember-concurrency'; import {getSymbol} from 'ghost-admin/utils/currency'; import {inject} from 'ghost-admin/decorators/inject'; import {inject as service} from '@ember/service'; import {slugify} from '@tryghost/string'; -import {task} from 'ember-concurrency'; import {timeout} from 'ember-concurrency'; import {tracked} from '@glimmer/tracking'; @@ -258,7 +258,16 @@ export default class OffersController extends Controller { @action setup() { - this.fetchTiers.perform(); + try { + this.fetchTiers.perform(); + } catch (e) { + // Do not throw cancellation errors + if (didCancel(e)) { + return; + } + + throw e; + } } @action diff --git a/ghost/admin/app/routes/members.js b/ghost/admin/app/routes/members.js index 67207e2620..235ed2d67d 100644 --- a/ghost/admin/app/routes/members.js +++ b/ghost/admin/app/routes/members.js @@ -1,4 +1,5 @@ import AdminRoute from 'ghost-admin/routes/admin'; +import {didCancel} from 'ember-concurrency'; import {inject as service} from '@ember/service'; export default class MembersRoute extends AdminRoute { @@ -27,7 +28,17 @@ export default class MembersRoute extends AdminRoute { // trigger a background load of members plus labels for filter dropdown setupController(controller) { super.setupController(...arguments); - controller.fetchLabelsTask.perform(); + + try { + controller.fetchLabelsTask.perform(); + } catch (e) { + // Do not throw cancellation errors + if (didCancel(e)) { + return; + } + + throw e; + } } resetController(controller, _isExiting, transition) {