feat: added custom status code for a missed block

[frolvanya]

As requested from our partner, custom status code for a missing block was added. I decided to go with 422 Unprocessable Content which seems pretty logical to me, but let me know if this is a bad choice

closes #12399

[walnut-the-cat]

hmm is UNKNOWN_BLOCK right one? From what i know, in nearcore, UNKNOWN_CHUNK is used when a node receives a chunk for the future block. So in that sense, UNKNOWN_BLOCK sounds like something similar (e.g. orphaned block). Maybe the meaning is different with RPC?

[telezhnaya]

Can we test it somehow?

[frolvanya]

hmm is UNKNOWN_BLOCK right one? From what i know, in nearcore, UNKNOWN_CHUNK is used when a node receives a chunk for the future block. So in that sense, UNKNOWN_BLOCK sounds like something similar (e.g. orphaned block). Maybe the meaning is different with RPC?

I mean "UNKNOWN_BLOCK" is a part of a handler error which is being returned when input block is either too low or too high

I tried to manually find a "missing" block to test it on, but I was out of luck

[codecov]

Codecov Report

Attention: Patch coverage is 79.16667% with 5 lines in your changes missing coverage. Please review.

Project coverage is 71.69%. Comparing base (cefc24d) to head (7c0584b).
Report is 15 commits behind head on master.

Files with missing lines	Patch %	Lines
chain/jsonrpc/src/lib.rs	79.16%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #12402      +/-   ##
==========================================
+ Coverage   71.62%   71.69%   +0.06%     
==========================================
  Files         842      843       +1     
  Lines      170738   170807      +69     
  Branches   170738   170807      +69     
==========================================
+ Hits       122299   122458     +159     
+ Misses      43058    42988      -70     
+ Partials     5381     5361      -20     

Flag	Coverage Δ
backward-compatibility	0.16% <0.00%> (+<0.01%)	⬆️
db-migration	0.16% <0.00%> (+<0.01%)	⬆️
genesis-check	1.27% <0.00%> (+<0.01%)	⬆️
integration-tests	39.36% <75.00%> (+0.18%)	⬆️
linux	70.98% <79.16%> (+0.35%)	⬆️
linux-nightly	71.27% <79.16%> (+0.07%)	⬆️
macos	50.64% <20.83%> (+0.04%)	⬆️
pytests	1.57% <0.00%> (+<0.01%)	⬆️
sanity-checks	1.38% <0.00%> (+<0.01%)	⬆️
unittests	64.25% <20.83%> (+0.01%)	⬆️
upgradability	0.21% <0.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nhovsmith-cb]

@frolvanya From explorer:block#131,402,418 observe that the parent is explorer:block#131,402,416. The network seemed to have skipped # 131,402,417

[frolvanya]

@frolvanya From explorer:block#131,402,418 observe that the parent is explorer:block#131,402,416. The network seemed to have skipped # 131,402,417

Thanks for the info! Then I have 2 updates:

1. Error is correct ("UNKNOWN_BLOCK")
2. "cause"."info" field is empty for some reason and that means that my logic won't work there. I'll try to modify current implementation tomorrow

[bowenwang1996]

@frolvanya could you add a unit test so that we know if something breaks it in the future?

[frolvanya]

@frolvanya could you add a unit test so that we know if something breaks it in the future?

Sure, I'm currently working on it

[xingyaow]

Hi team, can we get an ETA of when will this PR gets merged? Currently waiting for this new status code to unblock us, many thanks!

[walnut-the-cat]

Hi @xingyaow, sine this is nearcore fix, even when it is merged, it won't be automatically deployed. It needs to go through protocol version upgrade (either major or minor). By default, after the PR is merged in, it becomes candidate for 2.4 release, which is scheduled in mid December.

If it is very urgent from your end, we can try to prepare upgrade and release early next week.

Let us know-

[xingyaow]

Thanks, @walnut-the-cat! Yes it's urgent from our side as it's blocking us to launch Near by the end of December. We can't wait til mid December as we will have code freeze in December. Much appreciate it if this can be deployed and released sooner!

[walnut-the-cat]

@xingyaow ,

We will try that. In the meantime, this PR is basically code complete and missing part is thorough testing. @frolvanya thinks this should not cause any major issue even in the worst case, since it does not change any RPC logic.

If you are in hurry, it should be fairly safe for you to apply the PR on your RPC and unblock yourself for timebeing (Then later replace with proper binary)