Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: handle errors in Alchemy response for getWalletNfts #230

Merged
merged 47 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
77bf96c
test: initial fixtures for nft arrays
patricio0312rev Oct 13, 2023
edbd6ee
feat: log and add flag in case of error
patricio0312rev Oct 13, 2023
b60d990
test: AlchemyPendingRequest
patricio0312rev Oct 13, 2023
f89a9c2
fix: update the use of hasError in other implementations
patricio0312rev Oct 13, 2023
f035503
chore: update types
patricio0312rev Oct 13, 2023
f08b598
feat: support for hasError in Web3NftData
patricio0312rev Oct 13, 2023
fdaff51
style: resolve style guide violations
patricio0312rev Oct 13, 2023
651ab55
fix: Web3NftData
patricio0312rev Oct 13, 2023
c388aac
Merge branch 'feat/handle-alchemy-nft-uri-error' of https://github.co…
patricio0312rev Oct 13, 2023
ab756ef
fix: tests count
patricio0312rev Oct 13, 2023
889fdfd
fix: nft count for updated tests
patricio0312rev Oct 13, 2023
ff14545
fix: restore phpunit
patricio0312rev Oct 13, 2023
c75ade0
fix: AlchemyWeb3DataProvider
patricio0312rev Oct 13, 2023
16333b9
fix: getWalletNfts test in AlchemyWeb3DataProvider
patricio0312rev Oct 13, 2023
24f2013
test: coverage for line 702 in AlchemyPendingRequest
patricio0312rev Oct 16, 2023
bb6a9d6
style: resolve style guide violations
patricio0312rev Oct 16, 2023
45cf769
feat: add collection address to log
patricio0312rev Oct 16, 2023
0d4d113
test: coverage for line 702 in AlchemyPendingRequest
patricio0312rev Oct 16, 2023
91ace27
Merge branch 'develop' into feat/handle-alchemy-nft-uri-error
patricio0312rev Oct 16, 2023
d6d9dee
feat: add error column to nfts table
patricio0312rev Oct 17, 2023
189dded
feat: error field for Web3NftData
patricio0312rev Oct 17, 2023
d8dd2af
feat: enum for nft errors
patricio0312rev Oct 17, 2023
5a69bff
feat: set metadata outdated error if nft has no metadata
patricio0312rev Oct 17, 2023
fe21157
fix: implementations of Web3NftData
patricio0312rev Oct 17, 2023
3f8918b
test: nft array with errors and empty metadata object
patricio0312rev Oct 17, 2023
7507980
fix: implementations of Web3NftData
patricio0312rev Oct 17, 2023
0219c9d
style: resolve style guide violations
patricio0312rev Oct 17, 2023
fef68bf
feat: store error column for nft in Web3NftHandler
patricio0312rev Oct 17, 2023
733edef
fix: use of NftErrors enum
patricio0312rev Oct 17, 2023
11ee6c9
test: Web3NftHandler
patricio0312rev Oct 17, 2023
b18a74f
fix: remove reverse migration
patricio0312rev Oct 18, 2023
53f3fae
feat: rename NftErrors to NftInfo
patricio0312rev Oct 18, 2023
031f018
feat: rename migration to create info column
patricio0312rev Oct 18, 2023
9a2dd91
feat: update Web3NftData structure
patricio0312rev Oct 18, 2023
ce8595e
chore: update typing
patricio0312rev Oct 18, 2023
69abde4
fix: update AlchemyPendingRequest imports
patricio0312rev Oct 18, 2023
85de6ee
fix: Web3NftHandlerTest
patricio0312rev Oct 18, 2023
3485da3
fix: AlchemyPendingRequestTest
patricio0312rev Oct 18, 2023
39d5781
chore: rename error atribute to info
patricio0312rev Oct 18, 2023
980c3bf
feat: update logic to store info field in db
patricio0312rev Oct 18, 2023
e4c369f
Merge branch 'develop' into feat/handle-alchemy-nft-uri-error
patricio0312rev Oct 18, 2023
b7f1a64
fix: rector issues
patricio0312rev Oct 18, 2023
954a0bb
Merge branch 'develop' into feat/handle-alchemy-nft-uri-error
shahin-hq Oct 18, 2023
1a5510d
fix: update collectionName in parseNft
patricio0312rev Oct 19, 2023
f183ee3
Merge branch 'develop' into feat/handle-alchemy-nft-uri-error
goga-m Oct 19, 2023
a40c8c0
Update tests/App/Http/Client/Alchemy/AlchemyPendingRequestTest.php
patricio0312rev Oct 19, 2023
2249c63
fix: update conditions for setting nft info
patricio0312rev Oct 19, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/Data/Web3/Web3NftData.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ public function __construct(
public array $traits,
public int $mintedBlock,
public ?Carbon $mintedAt,
public ?bool $hasError,
public ?string $error,
) {
}

Expand Down
10 changes: 10 additions & 0 deletions app/Enums/NftErrors.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

namespace App\Enums;

enum NftErrors: string
{
case MetadataOutdated = 'METADATA_OUTDATED';
}
32 changes: 27 additions & 5 deletions app/Http/Client/Alchemy/AlchemyPendingRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use App\Enums\Chains;
use App\Enums\CryptoCurrencyDecimals;
use App\Enums\ImageSize;
use App\Enums\NftErrors;
use App\Enums\TokenType;
use App\Enums\TraitDisplayType;
use App\Exceptions\ConnectionException;
Expand All @@ -33,6 +34,7 @@
use Illuminate\Http\Client\Response;
use Illuminate\Support\Arr;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Str;
use stdClass;
use Throwable;
Expand Down Expand Up @@ -208,7 +210,7 @@ public function getWalletNfts(Wallet $wallet, Network $network, string $cursor =
$ownedNfts = Arr::get($data, 'ownedNfts', []);

$nfts = collect($ownedNfts)
->filter(fn ($nft) => $this->filterNft($nft))
->filter(fn ($nft) => $this->filterNft($nft, false))
->map(fn ($nft) => $this->parseNft($nft, $network->id))
->values();

Expand Down Expand Up @@ -364,7 +366,7 @@ public function getContractMetadataBatch(array $contactAddresses, Network $netwo
public function parseNft(array $nft, int $networkId, bool $convertTokenNumber = true): Web3NftData
{
$extractedFloorPrice = $this->tryExtractFloorPrice($nft);
$collectionName = $this->collectionName($nft);
$collectionName = $this->collectionName($nft) ?? Arr::get($nft, 'contractMetadata.symbol');
$description = $nft['description'] ?? null;
$supply = Arr::get($nft, 'contractMetadata.totalSupply');
if (is_numeric($supply)) {
Expand Down Expand Up @@ -395,11 +397,29 @@ public function parseNft(array $nft, int $networkId, bool $convertTokenNumber =

$tokenNumber = $convertTokenNumber === true ? CryptoUtils::hexToBigIntStr($nft['id']['tokenId']) : $nft['id']['tokenId'];

$error = Arr::get($nft, 'error');
$nftError = null;
$collectionAddress = Arr::get($nft, 'contract.address');

if (! empty($error)) {
Log::info('AlchemyPendingRequest: Filter NFT', [
patricio0312rev marked this conversation as resolved.
Show resolved Hide resolved
'error' => $error,
'collection_name' => $collectionName,
'collection_address' => $collectionAddress,
'nft_id' => $tokenNumber,
]);

// if metadata stuff is empty, is empty object or empty array
if (empty($nft['metadata']) || empty($nft['metadata']['metadata']) || is_object($nft['metadata']['metadata']) && count((array) $nft['metadata']['metadata'])) {
$nftError = NftErrors::MetadataOutdated->value;
}
}

return new Web3NftData(
tokenAddress: $nft['contract']['address'],
tokenNumber: $tokenNumber,
networkId: $networkId,
collectionName: $collectionName ?? Arr::get($nft, 'contractMetadata.symbol'),
collectionName: $collectionName,
collectionSymbol: Arr::get($nft, 'contractMetadata.symbol') ?? $collectionName,
collectionImage: Arr::get($nft, 'contractMetadata.openSea.imageUrl') ?? Arr::get($nft, 'media.0.thumbnail') ?? Arr::get($nft, 'media.0.gateway'),
collectionWebsite: Arr::get($nft, 'contractMetadata.openSea.externalUrl') ?? Arr::get($nft, 'metadata.external_url'),
Expand All @@ -421,6 +441,8 @@ public function parseNft(array $nft, int $networkId, bool $convertTokenNumber =
traits: $this->extractTraits($nft),
mintedBlock: $nft['contractMetadata']['deployedBlockNumber'],
mintedAt: $mintTimestamp !== null ? Carbon::createFromTimestampMs($mintTimestamp) : null,
hasError: ! empty($error),
patricio0312rev marked this conversation as resolved.
Show resolved Hide resolved
error: $nftError,
patricio0312rev marked this conversation as resolved.
Show resolved Hide resolved
);
}

Expand Down Expand Up @@ -681,13 +703,13 @@ private function getNftV2ApiUrl(): string
return 'https://'.self::$apiUrlPlaceholder.'.g.alchemy.com/nft/v2/';
}

private function filterNft(mixed $nft): bool
private function filterNft(mixed $nft, bool $filterError = true): bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is filterError for? I see it used once here https://github.com/ArdentHQ/dashbrd/pull/230/files#diff-957272cbfdd990765850dc7924f1ac272f6ea71fb3403acbe633fe548369ebf6R213 but that doesn't make it clear to me when to pass that value 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that in AlchemyPendingRequest, getWalletNfts wasn't the only endpoint filtering the data they receive. For this reason, I made the filter optional and we just turn off the filter for getWalletNfts, as the ticket mentions 👀

{
if (Arr::get($nft, 'spamInfo.isSpam', false)) {
return false;
}

if (Arr::has($nft, 'error')) {
if (Arr::has($nft, 'error') && $filterError) {
return false;
}

Expand Down
2 changes: 2 additions & 0 deletions app/Http/Client/Moralis/MoralisPendingRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ public function getWalletNfts(Wallet $wallet, Network $network, ?string $cursor)
traits: [],
mintedBlock: (int) $nft['block_number_minted'],
mintedAt: null,
hasError: false,
error: null,
);
})->values();

Expand Down
2 changes: 2 additions & 0 deletions app/Services/Web3/Fake/FakeWeb3DataProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ public function getWalletNfts(Wallet $wallet, Network $network, string $cursor =
traits: [],
mintedBlock: random_int(1, 10000),
mintedAt: now(),
hasError: false,
error: null,
);
});

Expand Down
40 changes: 28 additions & 12 deletions app/Support/Web3NftHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ public function store(Collection $nfts, bool $dispatchJobs = false): void
$attributes['opensea_slug'] = $nftData->collectionOpenSeaSlug;
}

if ($nftData->hasError) {
$attributes = array_filter($attributes, function ($value) {
return $value !== null;
});
}

return [
$nftData->tokenAddress,
$nftData->networkId,
Expand Down Expand Up @@ -124,22 +130,32 @@ public function store(Collection $nfts, bool $dispatchJobs = false): void
return $this->shouldKeepNft($collection);
});

$valuesToUpsert = $nfts->map(fn ($nft) => [
'wallet_id' => $this->wallet?->id,
'collection_id' => $groupedByAddress->get(Str::lower($nft->tokenAddress))->id,
'token_number' => $nft->tokenNumber,
'name' => $nft->name,
'extra_attributes' => json_encode($nft->extraAttributes),
'deleted_at' => null,
'metadata_fetched_at' => $now,
])->toArray();
$valuesToUpsert = $nfts->map(function ($nft) use ($groupedByAddress, $now) {
patricio0312rev marked this conversation as resolved.
Show resolved Hide resolved
$collection = $groupedByAddress->get(Str::lower($nft->tokenAddress));

$values = [
'wallet_id' => $this->wallet?->id,
'collection_id' => $collection->id,
'token_number' => $nft->tokenNumber,
'name' => $nft->name,
'extra_attributes' => json_encode($nft->extraAttributes),
'deleted_at' => null,
'metadata_fetched_at' => $now,
'error' => $nft->hasError ? $nft->error : null,
];

return $values;
})->toArray();

$uniqueBy = ['collection_id', 'token_number'];

$valuesToUpdateIfExists = ['name', 'extra_attributes', 'deleted_at', 'metadata_fetched_at'];
$valuesToUpdateIfExists = ['deleted_at', 'error'];
$valuesToCheck = ['name', 'extra_attributes', 'metadata_fetched_at', 'wallet_id'];

if ($this->wallet !== null) {
$valuesToUpdateIfExists[] = 'wallet_id';
foreach ($valuesToCheck as $value) {
if (array_filter($valuesToUpsert, fn ($v) => $v[$value] !== null)) {
$valuesToUpdateIfExists[] = $value;
}
}

Nft::upsert($valuesToUpsert, $uniqueBy, $valuesToUpdateIfExists);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

declare(strict_types=1);

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

return new class extends Migration
{
/**
* Run the migrations.
*/
public function up(): void
{
Schema::table('nfts', function (Blueprint $table) {
$table->string('error')->nullable();
});
}

/**
* Reverse the migrations.
*/
public function down(): void
{
Schema::table('nfts', function (Blueprint $table) {
patricio0312rev marked this conversation as resolved.
Show resolved Hide resolved
$table->dropColumn('error');
});
}
};
2 changes: 2 additions & 0 deletions database/seeders/LiveUserSeeder.php
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ traits: array_map(fn ($trait) => [
], $nft['traits']),
mintedAt: Carbon::parse($nft['mintedAt']),
mintedBlock: $nft['mintedBlock'],
hasError: false,
error: null,
));
}

Expand Down
2 changes: 2 additions & 0 deletions resources/types/generated.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,8 @@ declare namespace App.Data.Web3 {
traits: Array<any>;
mintedBlock: number;
mintedAt: string | null;
hasError: boolean | null;
error: string | null;
};
export type Web3NftTransfer = {
contractAddress: string;
Expand Down
78 changes: 78 additions & 0 deletions tests/App/Http/Client/Alchemy/AlchemyPendingRequestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

declare(strict_types=1);

use App\Enums\NftErrors;
use App\Exceptions\ConnectionException;
use App\Exceptions\RateLimitException;
use App\Models\Collection;
Expand Down Expand Up @@ -94,3 +95,80 @@

expect($fetchedNfts->nfts)->toHaveCount(1);
});

it('should add a flag in case of responses with errors', function () {
Alchemy::fake([
'https://polygon-mainnet.g.alchemy.com/nft/v2/*' => Http::sequence()
->push(fixtureData('alchemy.nfts_array_with_error'), 200),
]);

$wallet = Wallet::factory()->create();
$network = Network::polygon();

$collection = Alchemy::getWalletNfts($wallet, $network);
expect($collection->nfts[0]->hasError)->toBetrue();
});

it('should not add a flag in case of valid responses', function () {
Alchemy::fake([
'https://polygon-mainnet.g.alchemy.com/nft/v2/*' => Http::sequence()
->push(fixtureData('alchemy.nfts_array_valid'), 200),
]);

$wallet = Wallet::factory()->create();
$network = Network::polygon();

$collection = Alchemy::getWalletNfts($wallet, $network);
expect($collection->nfts[0]->hasError)->not->toBeTrue();
});

it('should not filter nfts with errors if the flag is set as true', function () {
Alchemy::fake([
'https://polygon-mainnet.g.alchemy.com/nft/v2/*' => Http::sequence()
->push(fixtureData('alchemy.nfts_array_with_error'), 200),
]);

$wallet = Wallet::factory()->create();
$network = Network::polygon();

$collection = Alchemy::getWalletNfts($wallet, $network);
expect($collection->nfts)->toHaveCount(1);
});

it('should filter nfts with errors by default', function () {
$user = createUser();

$network = Network::polygon();
$collection = Collection::factory()->create(['network_id' => $network->id]);

$nfts = collect();
$nft = Nft::factory()->create([
'wallet_id' => $user->wallet,
'collection_id' => $collection,
]);

$nfts->push($nft);

Alchemy::fake([
'https://polygon-mainnet.g.alchemy.com/nft/v2/*' => Http::response(fixtureData('alchemy.nft_batch_metadata_with_error'), 200),
]);

$fetchedNfts = Alchemy::nftMetadataBatch($nfts, $network);

expect($fetchedNfts->nfts)->toHaveCount(0);
});

it('should return error field with METADATA_OUTATED if metadata object is empty', function () {
patricio0312rev marked this conversation as resolved.
Show resolved Hide resolved
Alchemy::fake([
'https://polygon-mainnet.g.alchemy.com/nft/v2/*' => Http::sequence()
->push(fixtureData('alchemy.nfts_array_with_error_and_empty_metadata_object'), 200),
]);

$wallet = Wallet::factory()->create();
$network = Network::polygon();

$collection = Alchemy::getWalletNfts($wallet, $network);
print_r($collection);
expect($collection->nfts[0]->hasError)->toBetrue();
expect($collection->nfts[0]->error)->toBe(NftErrors::MetadataOutdated->value);
});
8 changes: 8 additions & 0 deletions tests/App/Jobs/DetermineCollectionMintingDateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@
traits: [],
mintedBlock: 1000,
mintedAt: null,
hasError: false,
error: null,
);

$job = new DetermineCollectionMintingDate($nft);
Expand Down Expand Up @@ -103,6 +105,8 @@ traits: [],
traits: [],
mintedBlock: 1000,
mintedAt: null,
hasError: false,
error: null,
);

$job = new DetermineCollectionMintingDate($nft);
Expand Down Expand Up @@ -136,6 +140,8 @@ traits: [],
traits: [],
mintedBlock: 1000,
mintedAt: null,
hasError: false,
error: null,
);

$job = new DetermineCollectionMintingDate($nft);
Expand Down Expand Up @@ -167,6 +173,8 @@ traits: [],
traits: [],
mintedBlock: 1000,
mintedAt: null,
hasError: false,
error: null,
);

$job = new DetermineCollectionMintingDate($nft);
Expand Down
Loading
Loading