Better error handling for no builder bids on /eth/v1/builder/header

[philknows]

Describe the bug

Currently, we always assume that we receive a JSON body from the /eth/v1/builder/header API. When using --builder and mev-boost returns no bids received, we get a 204 (no content) response and it triggers Unexpected end of JSON input on Lodestar.

Expected behavior

We should have a more graceful response to no bids. It should not show up as an error: and should likely be downgraded to either warn: or lower in accordance with new error logging policy #5359 implemented by #5299. The stacktrace is probably better moved to debug or verbose as it should feel less intrusive to not receive bids or not meet the min-bid threshold.

Steps to reproduce

No response

Additional context

Example (beacon-node):

Jul-23 12:07:02.474[rest]            error: Req req-46b8 produceBlindedBlock error  invalid json response body at http://127.0.0.1:18550/eth/v1/builder/header/XXXXXXX/0x12345abcdef/0x12345abcdef reason: Unexpected end of JSON input
2023-07-23 12:07:02    
FetchError: invalid json response body at http://127.0.0.1:18550/eth/v1/builder/header/XXXXXXX/0x12345abcdef/0x12345abcdef reason: Unexpected end of JSON input
2023-07-23 12:07:02    
    at /usr/app/node_modules/node-fetch/lib/index.js:272:32
2023-07-23 12:07:02    
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
2023-07-23 12:07:02    
    at HttpClient.requestWithBody (file:///usr/app/packages/api/src/utils/client/httpClient.ts:298:20)
2023-07-23 12:07:02    
    at Object.request [as getHeader] (file:///usr/app/packages/api/src/utils/client/client.ts:69:23)
2023-07-23 12:07:02    
    at ExecutionBuilderHttp.getHeader (file:///usr/app/packages/beacon-node/src/execution/builder/http.ts:99:17)
2023-07-23 12:07:02    
    at BeaconChain.produceBlockBody (file:///usr/app/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts:167:26)
2023-07-23 12:07:02    
    at BeaconChain.produceBlockWrapper (file:///usr/app/packages/beacon-node/src/chain/chain.ts:495:39)
2023-07-23 12:07:02    
    at Object.produceBlindedBlock (file:///usr/app/packages/beacon-node/src/api/impl/validator/index.ts:220:37)
2023-07-23 12:07:02    
    at Object.handler (file:///usr/app/packages/api/src/utils/server/genericJsonServer.ts:45:23)

Example mev-boost:

2023-07-23 12:07:01    
time="2023-07-23T12:07:01.904Z" level=info msg="getHeader request start - 14904 milliseconds into slot XXXXXXX" genesisTime=1606824023 method=getHeader msIntoSlot=14904 parentHash=0x123456 pubkey=0xabcdef slot=XXXXXXX slotTimeSec=12 slotUID=123-456-789-abcdef ua=Lodestar/v1.9.2/72beda1 version=v1.6
2023-07-23 12:07:02    
time="2023-07-23T12:07:02.13Z" level=info msg="no bid received" method=getHeader parentHash= pubkey=0xabcdef slot=XXXXXXX slotUID=123-456-789-abcdef ua=Lodestar/v1.9.2/72beda1 version=v1.6
2023-07-23 12:07:02    
time="2023-07-23T12:07:02.13Z" level=info msg="http: GET /eth/v1/builder/header/XXXXXXX/0x12345abcdef 204" duration=0.225498 method=GET path=/eth/v1/builder/header/XXXXXXX//0x12345abcdef/0x12345abcdef status=204 version=v1.6

Operating system

Linux

Lodestar version or commit hash

v1.9.2

[philknows]

bump

• Edit: this should be fixed in the API refactor in API SSZ response #5128

[nflaig]

Edit: this should be fixed in the API refactor in #5128

Doesn't fully address this but it gives us more control on when to parse the request body (res.json()), we still need to implement the handling of the 204 response which is out of scope of the ssz refactoring but rather simple to implement, see #6574 (comment).

[nflaig]

Current solution I have implemented is to throw an error explicitly if we don't received a bid

lodestar/packages/beacon-node/src/execution/builder/http.ts

Lines 108 to 112 in fd482c2

	const signedBuilderBid = (await this.api.getHeader({slot, parentHash, proposerPubkey})).value();
	if (!signedBuilderBid) {
	throw Error("No bid received");
	}

In produceBlockV3 this would result in a warn: log, we would only log a error: if local block also fails to produce, which seems justified.

[nflaig]

Alternative would be to not throw an error and propagate null up the call stack until engine vs. builder race is resolved and more gracefully handle it there.

[philknows]

Generally when I see error: on my logs, it signals to me as the user that I need to do something to intervene. If I'm not producing a local block also, then yes I think it is justified to prompt the user with an error to fix that connection. Ideally that should be a last resort and for other cases, we should handle more it more gracefully if possible.