Add FILTER_TRANSACTION_NOT_FOUND error#1681
Conversation
🦋 Changeset detectedLatest commit: 2ea6a1c The changes in this PR will be included in the next version bump. This PR includes changesets to release 47 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
BundleMonFiles updated (7)
Unchanged files (140)
Total files change +218B +0.04% Final result: ✅ View report in BundleMon website ➡️ |
brooksprumo
left a comment
There was a problem hiding this comment.
I also saw that a changeset was added to #1679, but I haven't done that here yet.
| [SOLANA_ERROR__JSON_RPC__SERVER_ERROR_BLOCK_STATUS_NOT_AVAILABLE_YET]: '$__serverMessage', | ||
| [SOLANA_ERROR__JSON_RPC__SERVER_ERROR_EPOCH_REWARDS_PERIOD_ACTIVE]: | ||
| 'Epoch rewards period still active at slot $slot', | ||
| [SOLANA_ERROR__JSON_RPC__SERVER_ERROR_FILTER_TRANSACTION_NOT_FOUND]: '$__serverMessage', |
There was a problem hiding this comment.
I wasn't sure what to put here, so guessed based on similar json rpc errors that contained a String message and no data (e.g. KEY_EXCLUDED_FROM_SECONDARY_INDEX). Please double check 😸
|
@lorisleiva @joncinque - tagging y'all again for a review request. Thanks in advance! |
lorisleiva
left a comment
There was a problem hiding this comment.
Thanks! IIRC, you need to add this error code here to add the server message in the context:
kit/packages/errors/src/json-rpc-error.ts
Lines 121 to 140 in 74b8d3d
|
Oh, I just saw https://github.com/anza-xyz/kit/tree/main/packages/errors/#adding-a-new-error. Should I also add to Edit: I've added to |
416f2d8 to
2ea6a1c
Compare
| rewardsCompleteBlockHeight: bigint; | ||
| slot: bigint; | ||
| }; | ||
| [SOLANA_ERROR__JSON_RPC__SERVER_ERROR_FILTER_TRANSACTION_NOT_FOUND]: { |
There was a problem hiding this comment.
Ah yeah good shout! I think you've just forgotten to import the constant in this file which is why CI is failing.
Btw, am I right to say your previous error code had no context? That is, no data attached to the error on the RPC server.
There was a problem hiding this comment.
Yep, good find. Should be fixed now.
And yes, the error does not have anything in the data field, only the message field.
RpcCustomError::FilterTransactionNotFound { signature } => Self {
code: ErrorCode::ServerError(JSON_RPC_SERVER_ERROR_FILTER_TRANSACTION_NOT_FOUND),
message: format!("Transaction {signature} not found"),
data: None,
},There was a problem hiding this comment.
Oh, did you mean the NO_SLOT_HISTORY error?
RpcCustomError::NoSlotHistory => Self {
code: ErrorCode::ServerError(JSON_RPC_SERVER_ERROR_NO_SLOT_HISTORY),
message: "No slot history".to_string(),
data: None,
},There was a problem hiding this comment.
Yeah that's all good then! The server message trick is to ensure we don't loose any dynamic data only present in the error message. In the case of JSON_RPC_SERVER_ERROR_FILTER_TRANSACTION_NOT_FOUND, that's the signature inside the message.
On that note, may I ask why we're not returning that signature as data? Then, consumers could access that signature directly instead of having to parse the error message.
There was a problem hiding this comment.
On that note, may I ask why we're not returning that
signatureas data?
No idea. Probably because we didn't know to do that 😅
There was a problem hiding this comment.
Has this change landed already server side? Not a big deal if it has but it would be nice if moving forward we could make sure dynamic error information is provided as data instead of available in the error message only. Ideally we would even retroactively fix these errors (as per the following comment in Kit) but I realise this may be near impossible.
// The server supplies no structured data, but rather a pre-formatted message. Put
// the server message in `context` so as not to completely lose the data. The long
// term fix for this is to add data to the server responses and modify the
// messages in `@solana/errors` to be actual format strings.
In any case, LGTM and we can always work on a subsequent PR if the server side can still be changed.
There was a problem hiding this comment.
Looks like the original agave PR was backported to v4.0, so it is going live now.
Re:
dynamic error information is provided as data instead of available in the error message only
Yes, I agree.
Problem
When working on #1679, I noticed Kit was a missing json rpc error. It was added to agave in anza-xyz/agave#7937.
Summary of Changes
Add it to Kit.