From 5922ff0f40e526b7c52ede7bc43845cd6280fc13 Mon Sep 17 00:00:00 2001 From: Mononaut Date: Wed, 31 Aug 2022 17:24:56 +0000 Subject: [PATCH 1/2] Better fix for unfurler race condition --- .../address/address-preview.component.ts | 10 ++++++---- .../block/block-preview.component.ts | 20 ++++++++++--------- .../transaction-preview.component.ts | 17 ++++++++-------- .../channel/channel-preview.component.ts | 14 +++++++------ .../lightning/node/node-preview.component.ts | 14 ++++++------- .../src/app/services/opengraph.service.ts | 8 ++++---- unfurler/src/concurrency/ReusablePage.ts | 4 ++++ unfurler/src/index.ts | 15 +++++++------- 8 files changed, 56 insertions(+), 46 deletions(-) diff --git a/frontend/src/app/components/address/address-preview.component.ts b/frontend/src/app/components/address/address-preview.component.ts index c0f6fff81..35e72435b 100644 --- a/frontend/src/app/components/address/address-preview.component.ts +++ b/frontend/src/app/components/address/address-preview.component.ts @@ -19,6 +19,7 @@ import { AddressInformation } from 'src/app/interfaces/node-api.interface'; export class AddressPreviewComponent implements OnInit, OnDestroy { network = ''; + rawAddress: string; address: Address; addressString: string; isLoadingAddress = true; @@ -55,7 +56,8 @@ export class AddressPreviewComponent implements OnInit, OnDestroy { this.mainSubscription = this.route.paramMap .pipe( switchMap((params: ParamMap) => { - this.openGraphService.waitFor('address-data'); + this.rawAddress = params.get('id') || ''; + this.openGraphService.waitFor('address-data-' + this.rawAddress); this.error = undefined; this.isLoadingAddress = true; this.loadedConfirmedTxCount = 0; @@ -73,7 +75,7 @@ export class AddressPreviewComponent implements OnInit, OnDestroy { this.isLoadingAddress = false; this.error = err; console.log(err); - this.openGraphService.fail('address-data'); + this.openGraphService.fail('address-data-' + this.rawAddress); return of(null); }) ); @@ -91,7 +93,7 @@ export class AddressPreviewComponent implements OnInit, OnDestroy { this.address = address; this.updateChainStats(); this.isLoadingAddress = false; - this.openGraphService.waitOver('address-data'); + this.openGraphService.waitOver('address-data-' + this.rawAddress); }) ) .subscribe(() => {}, @@ -99,7 +101,7 @@ export class AddressPreviewComponent implements OnInit, OnDestroy { console.log(error); this.error = error; this.isLoadingAddress = false; - this.openGraphService.fail('address-data'); + this.openGraphService.fail('address-data-' + this.rawAddress); } ); } diff --git a/frontend/src/app/components/block/block-preview.component.ts b/frontend/src/app/components/block/block-preview.component.ts index f1c7216e1..6b96887f0 100644 --- a/frontend/src/app/components/block/block-preview.component.ts +++ b/frontend/src/app/components/block/block-preview.component.ts @@ -20,6 +20,7 @@ export class BlockPreviewComponent implements OnInit, OnDestroy { block: BlockExtended; blockHeight: number; blockHash: string; + rawId: string; isLoadingBlock = true; strippedTransactions: TransactionStripped[]; overviewTransitionDirection: string; @@ -48,8 +49,9 @@ export class BlockPreviewComponent implements OnInit, OnDestroy { const block$ = this.route.paramMap.pipe( switchMap((params: ParamMap) => { - this.openGraphService.waitFor('block-viz'); - this.openGraphService.waitFor('block-data'); + this.rawId = params.get('id') || ''; + this.openGraphService.waitFor('block-viz-' + this.rawId); + this.openGraphService.waitFor('block-data-' + this.rawId); const blockHash: string = params.get('id') || ''; this.block = undefined; @@ -80,8 +82,8 @@ export class BlockPreviewComponent implements OnInit, OnDestroy { }), catchError((err) => { this.error = err; - this.openGraphService.fail('block-data'); - this.openGraphService.fail('block-viz'); + this.openGraphService.fail('block-data-' + this.rawId); + this.openGraphService.fail('block-viz-' + this.rawId); return of(null); }), ); @@ -103,7 +105,7 @@ export class BlockPreviewComponent implements OnInit, OnDestroy { this.isLoadingOverview = true; this.overviewError = null; - this.openGraphService.waitOver('block-data'); + this.openGraphService.waitOver('block-data-' + this.rawId); }), throttleTime(50, asyncScheduler, { leading: true, trailing: true }), shareReplay(1) @@ -116,7 +118,7 @@ export class BlockPreviewComponent implements OnInit, OnDestroy { .pipe( catchError((err) => { this.overviewError = err; - this.openGraphService.fail('block-viz'); + this.openGraphService.fail('block-viz-' + this.rawId); return of([]); }), switchMap((transactions) => { @@ -136,8 +138,8 @@ export class BlockPreviewComponent implements OnInit, OnDestroy { (error) => { this.error = error; this.isLoadingOverview = false; - this.openGraphService.fail('block-viz'); - this.openGraphService.fail('block-data'); + this.openGraphService.fail('block-viz-' + this.rawId); + this.openGraphService.fail('block-data-' + this.rawId); if (this.blockGraph) { this.blockGraph.destroy(); } @@ -163,6 +165,6 @@ export class BlockPreviewComponent implements OnInit, OnDestroy { } onGraphReady(): void { - this.openGraphService.waitOver('block-viz'); + this.openGraphService.waitOver('block-viz-' + this.rawId); } } diff --git a/frontend/src/app/components/transaction/transaction-preview.component.ts b/frontend/src/app/components/transaction/transaction-preview.component.ts index d30789f6b..040200cf6 100644 --- a/frontend/src/app/components/transaction/transaction-preview.component.ts +++ b/frontend/src/app/components/transaction/transaction-preview.component.ts @@ -92,15 +92,16 @@ export class TransactionPreviewComponent implements OnInit, OnDestroy { txFeePerVSize: this.tx.effectiveFeePerVsize, }); this.cpfpInfo = cpfpInfo; - this.openGraphService.waitOver('cpfp-data'); + this.openGraphService.waitOver('cpfp-data-' + this.txId); }); this.subscription = this.route.paramMap .pipe( switchMap((params: ParamMap) => { - this.openGraphService.waitFor('tx-data'); const urlMatch = (params.get('id') || '').split(':'); this.txId = urlMatch[0]; + this.openGraphService.waitFor('tx-data-' + this.txId); + this.openGraphService.waitFor('tx-time-' + this.txId); this.seoService.setTitle( $localize`:@@bisq.transaction.browser-title:Transaction: ${this.txId}:INTERPOLATION:` ); @@ -149,7 +150,7 @@ export class TransactionPreviewComponent implements OnInit, OnDestroy { ) .subscribe((tx: Transaction) => { if (!tx) { - this.openGraphService.fail('tx-data'); + this.openGraphService.fail('tx-data-' + this.txId); return; } @@ -166,6 +167,7 @@ export class TransactionPreviewComponent implements OnInit, OnDestroy { if (!tx.status.confirmed && tx.firstSeen) { this.transactionTime = tx.firstSeen; + this.openGraphService.waitOver('tx-time-' + this.txId); } else { this.getTransactionTime(); } @@ -177,15 +179,15 @@ export class TransactionPreviewComponent implements OnInit, OnDestroy { bestDescendant: tx.bestDescendant, }; } else { - this.openGraphService.waitFor('cpfp-data'); + this.openGraphService.waitFor('cpfp-data-' + this.txId); this.fetchCpfp$.next(this.tx.txid); } } - this.openGraphService.waitOver('tx-data'); + this.openGraphService.waitOver('tx-data-' + this.txId); }, (error) => { - this.openGraphService.fail('tx-data'); + this.openGraphService.fail('tx-data-' + this.txId); this.error = error; this.isLoadingTx = false; } @@ -193,7 +195,6 @@ export class TransactionPreviewComponent implements OnInit, OnDestroy { } getTransactionTime() { - this.openGraphService.waitFor('tx-time'); this.apiService .getTransactionTimes$([this.tx.txid]) .pipe( @@ -203,7 +204,7 @@ export class TransactionPreviewComponent implements OnInit, OnDestroy { ) .subscribe((transactionTimes) => { this.transactionTime = transactionTimes[0]; - this.openGraphService.waitOver('tx-time'); + this.openGraphService.waitOver('tx-time-' + this.txId); }); } diff --git a/frontend/src/app/lightning/channel/channel-preview.component.ts b/frontend/src/app/lightning/channel/channel-preview.component.ts index c82adba66..10c57c085 100644 --- a/frontend/src/app/lightning/channel/channel-preview.component.ts +++ b/frontend/src/app/lightning/channel/channel-preview.component.ts @@ -16,6 +16,7 @@ export class ChannelPreviewComponent implements OnInit { channel$: Observable; error: any = null; channelGeo: number[] = []; + shortId: string; constructor( private lightningApiService: LightningApiService, @@ -28,8 +29,9 @@ export class ChannelPreviewComponent implements OnInit { this.channel$ = this.activatedRoute.paramMap .pipe( switchMap((params: ParamMap) => { - this.openGraphService.waitFor('channel-map'); - this.openGraphService.waitFor('channel-data'); + this.shortId = params.get('short_id') || ''; + this.openGraphService.waitFor('channel-map-' + this.shortId); + this.openGraphService.waitFor('channel-data-' + this.shortId); this.error = null; this.seoService.setTitle(`Channel: ${params.get('short_id')}`); return this.lightningApiService.getChannel$(params.get('short_id')) @@ -48,12 +50,12 @@ export class ChannelPreviewComponent implements OnInit { data.node_right.longitude, data.node_right.latitude, ]; } - this.openGraphService.waitOver('channel-data'); + this.openGraphService.waitOver('channel-data-' + this.shortId); }), catchError((err) => { this.error = err; - this.openGraphService.fail('channel-map'); - this.openGraphService.fail('channel-data'); + this.openGraphService.fail('channel-map-' + this.shortId); + this.openGraphService.fail('channel-data-' + this.shortId); return of(null); }) ); @@ -62,6 +64,6 @@ export class ChannelPreviewComponent implements OnInit { } onMapReady() { - this.openGraphService.waitOver('channel-map'); + this.openGraphService.waitOver('channel-map-' + this.shortId); } } diff --git a/frontend/src/app/lightning/node/node-preview.component.ts b/frontend/src/app/lightning/node/node-preview.component.ts index 0d6908eb1..c2c3ab72c 100644 --- a/frontend/src/app/lightning/node/node-preview.component.ts +++ b/frontend/src/app/lightning/node/node-preview.component.ts @@ -42,9 +42,9 @@ export class NodePreviewComponent implements OnInit { this.node$ = this.activatedRoute.paramMap .pipe( switchMap((params: ParamMap) => { - this.openGraphService.waitFor('node-map'); - this.openGraphService.waitFor('node-data'); - this.publicKey = params.get('public_key'); + this.publicKey = params.get('public_key'); + this.openGraphService.waitFor('node-map-' + this.publicKey); + this.openGraphService.waitFor('node-data-' + this.publicKey); return this.lightningApiService.getNode$(params.get('public_key')); }), map((node) => { @@ -75,14 +75,14 @@ export class NodePreviewComponent implements OnInit { this.socketTypes = Object.keys(socketTypesMap); node.avgCapacity = node.capacity / Math.max(1, node.active_channel_count); - this.openGraphService.waitOver('node-data'); + this.openGraphService.waitOver('node-data-' + this.publicKey); return node; }), catchError(err => { this.error = err; - this.openGraphService.fail('node-map'); - this.openGraphService.fail('node-data'); + this.openGraphService.fail('node-map-' + this.publicKey); + this.openGraphService.fail('node-data-' + this.publicKey); return [{ alias: this.publicKey, public_key: this.publicKey, @@ -100,6 +100,6 @@ export class NodePreviewComponent implements OnInit { } onMapReady() { - this.openGraphService.waitOver('node-map'); + this.openGraphService.waitOver('node-map-' + this.publicKey); } } diff --git a/frontend/src/app/services/opengraph.service.ts b/frontend/src/app/services/opengraph.service.ts index dc62db0f3..9e2fef781 100644 --- a/frontend/src/app/services/opengraph.service.ts +++ b/frontend/src/app/services/opengraph.service.ts @@ -83,13 +83,13 @@ export class OpenGraphService { waitOver(event) { if (this.previewLoadingEvents[event]) { this.previewLoadingEvents[event]--; - if (this.previewLoadingEvents[event] === 0) { + if (this.previewLoadingEvents[event] === 0 && this.previewLoadingCount > 0) { delete this.previewLoadingEvents[event] this.previewLoadingCount--; } - } - if (this.previewLoadingCount === 0) { - this.metaService.updateTag({ property: 'og:preview:ready', content: 'ready'}); + if (this.previewLoadingCount === 0) { + this.metaService.updateTag({ property: 'og:preview:ready', content: 'ready'}); + } } } diff --git a/unfurler/src/concurrency/ReusablePage.ts b/unfurler/src/concurrency/ReusablePage.ts index 9592ea702..ed400d004 100644 --- a/unfurler/src/concurrency/ReusablePage.ts +++ b/unfurler/src/concurrency/ReusablePage.ts @@ -87,6 +87,10 @@ export default class ReusablePage extends ConcurrencyImplementation { page.repairRequested = true; }); await page.goto(defaultUrl, { waitUntil: "load" }); + await Promise.race([ + page.waitForSelector('meta[property="og:preview:ready"]', { timeout: config.PUPPETEER.RENDER_TIMEOUT || 3000 }).then(() => true), + page.waitForSelector('meta[property="og:preview:fail"]', { timeout: config.PUPPETEER.RENDER_TIMEOUT || 3000 }).then(() => false) + ]) page.free = true; return page } diff --git a/unfurler/src/index.ts b/unfurler/src/index.ts index ff5e6963a..4ae1d088e 100644 --- a/unfurler/src/index.ts +++ b/unfurler/src/index.ts @@ -100,14 +100,13 @@ class Server { } } - const waitForReady = await page.$('meta[property="og:preview:loading"]'); - let success = true; - if (waitForReady != null) { - success = await Promise.race([ - page.waitForSelector('meta[property="og:preview:ready"]', { timeout: config.PUPPETEER.RENDER_TIMEOUT || 3000 }).then(() => true), - page.waitForSelector('meta[property="og:preview:fail"]', { timeout: config.PUPPETEER.RENDER_TIMEOUT || 3000 }).then(() => false) - ]) - } + // wait for preview component to initialize + await page.waitForSelector('meta[property="og:preview:loading"]', { timeout: config.PUPPETEER.RENDER_TIMEOUT || 3000 }) + let success = false; + success = await Promise.race([ + page.waitForSelector('meta[property="og:preview:ready"]', { timeout: config.PUPPETEER.RENDER_TIMEOUT || 3000 }).then(() => true), + page.waitForSelector('meta[property="og:preview:fail"]', { timeout: config.PUPPETEER.RENDER_TIMEOUT || 3000 }).then(() => false) + ]) if (success) { const screenshot = await page.screenshot(); return screenshot; From 80dfa0e937ba5a3f406548f97fdc0e8d3ad8ac64 Mon Sep 17 00:00:00 2001 From: Mononaut Date: Wed, 31 Aug 2022 18:21:24 +0000 Subject: [PATCH 2/2] Fix null timestamps on transaction previews --- .../transaction/transaction-preview.component.html | 2 +- .../components/transaction/transaction-preview.component.ts | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/frontend/src/app/components/transaction/transaction-preview.component.html b/frontend/src/app/components/transaction/transaction-preview.component.html index 44be1dcfa..1708d4b47 100644 --- a/frontend/src/app/components/transaction/transaction-preview.component.html +++ b/frontend/src/app/components/transaction/transaction-preview.component.html @@ -23,7 +23,7 @@ - ‎{{ (tx.status.confirmed ? tx.status.block_time : transactionTime) * 1000 | date:'yyyy-MM-dd HH:mm' }} + ‎{{ transactionTime * 1000 | date:'yyyy-MM-dd HH:mm' }} Fee {{ tx.fee | number }} sat diff --git a/frontend/src/app/components/transaction/transaction-preview.component.ts b/frontend/src/app/components/transaction/transaction-preview.component.ts index 040200cf6..914801e70 100644 --- a/frontend/src/app/components/transaction/transaction-preview.component.ts +++ b/frontend/src/app/components/transaction/transaction-preview.component.ts @@ -165,7 +165,10 @@ export class TransactionPreviewComponent implements OnInit, OnDestroy { this.opReturns = this.getOpReturns(this.tx); this.extraData = this.chooseExtraData(); - if (!tx.status.confirmed && tx.firstSeen) { + if (tx.status.confirmed) { + this.transactionTime = tx.status.block_time; + this.openGraphService.waitOver('tx-time-' + this.txId); + } else if (!tx.status.confirmed && tx.firstSeen) { this.transactionTime = tx.firstSeen; this.openGraphService.waitOver('tx-time-' + this.txId); } else {