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.

Sign up for GitHub

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

Jump to bottom

fix: prevent serde error from crashing service #1097

Merged
merged 2 commits into from
Jul 13, 2023
Merged

fix: prevent serde error from crashing service #1097

merged 2 commits into from
Jul 13, 2023

Conversation

ra0x3
Copy link
Contributor

@ra0x3 ra0x3 commented Jul 12, 2023 •
edited
Loading

  • Please add proper labels
  • If there is an issue associated with this PR, please link the issue (right-hand sidebar)
  • If there is not an issue associated with this PR, add this PR to the "Fuel Indexer" project (right-hand sidebar)

Description

  • Prevents from panic'ing when put_object fails.
    • Currently this crashes the entire service
    • Why does it fail? Not sure, but we can't bring down the entire service because of this
    • We successfully indexed 178,775 blocks before encountering this error - some type of blip?
  • Here is the elastic doc with the exact error

Testing steps

  • New indexer forc index new indexer-test --namespace fuel
  • Use 178775 as your start_block
  • Start the service on beta-3
cargo run -p fuel-indexer -- run --run-migrations --fuel-node-host beta-3.fuel.network --fuel-node-port 80 --replace-indexer
  • Deploy forc index deploy
  • Should see no panic

Changelog

  • fix: prevent serde error from crashing service

@ra0x3 ra0x3 added the bug Something isn't working label Jul 12, 2023
@ra0x3 ra0x3 self-assigned this Jul 12, 2023
@ra0x3 ra0x3 requested a review from deekerno as a code owner July 12, 2023 23:00
@ra0x3 ra0x3 marked this pull request as draft July 12, 2023 23:00
@ra0x3 ra0x3 linked an issue Jul 12, 2023 that may be closed by this pull request
Invalid encoded byte error on beta-3-indexer #1096
Closed
@ra0x3 ra0x3 marked this pull request as ready for review July 12, 2023 23:09
ra0x3
ra0x3 commented Jul 12, 2023
View reviewed changes
packages/fuel-indexer-macros/src/decoder.rs
@@ -673,14 +673,14 @@ impl From<ObjectDecoder> for TokenStream {

impl From<#ident> for Json {
fn from(value: #ident) -> Self {
let s = serde_json::to_string(&value).expect("Serde error.");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • All of these errors were "Serde error."
  • Hard to sort through this in the repo, so I made them unique.

ra0x3
ra0x3 commented Jul 12, 2023
View reviewed changes
packages/fuel-indexer/src/ffi.rs
@@ -154,7 +154,16 @@ fn put_object(env: &IndexEnv, type_id: i64, ptr: u32, len: u32) {
bytes.extend_from_slice(&mem.data_unchecked()[range]);
}

let columns: Vec<FtColumn> = bincode::deserialize(&bytes).expect("Serde error.");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is where the actual bug is. Not sure what causes 1 out of 178,778 of these to be incorrect but we can look at the data later. For now it just shouldn't crash the service.

@ra0x3 ra0x3 requested a review from lostman July 12, 2023 23:16
@ra0x3 ra0x3 merged commit c7159f1 into develop Jul 13, 2023
45 checks passed
@ra0x3 ra0x3 deleted the rashad/1096-fix-serde-error branch July 13, 2023 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deekerno deekerno deekerno approved these changes

@lostman lostman Awaiting requested review from lostman lostman is a code owner

Assignees

@ra0x3 ra0x3

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Invalid encoded byte error on beta-3-indexer
2 participants
@ra0x3 @deekerno