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
This commit is contained in:
Sag 2024-06-04 16:20:49 +02:00 committed by GitHub
parent 0efec254ec
commit 98b51b666d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 134 additions and 32 deletions

View File

@ -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

View File

@ -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);
};

View File

@ -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;

View File

@ -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

View File

@ -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

View File

@ -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)}`;

View File

@ -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'});
}

View File

@ -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

View File

@ -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) {