Use Messenger for RUN_BLOCK_* methods

[fregante]

• Replaces Use Messenger for executeInTarget #2044

Context

As mentioned in the previous PR, I'm replacing the previous sendMessage calls to Messenger, but the forwarding is done manually (due to custom logic), i.e. the reducer messages the background and then the background messages the desired target.

Naming

For consistency, I named these two messages differently:

• REQUEST_RUN -> towards the background
• RUN -> towards the content script

and also ensured that they're always called BRICK and not BLOCK.

Test status

• REQUEST_RUN_ON_SERVER: seen
• REQUEST_RUN_IN_OPENER: ✅ (video below)
• REQUEST_RUN_IN_TARGET: ✅
• REQUEST_RUN_IN_FRAME_NONCE: not yet tested
• REQUEST_RUN_IN_ALL: ✅

Refactoring

• The logic should be unchanged
• The tab readiness is also unchanged
• Broadcasting has been reduced to a single loop
• The rest of the handlers have been slightly adjusted to be consistent and avoid "payload", which no longer applies.
• Some logging has been removed because it's mostly covered by the messenger

Concerns

• retrySend was called in the source tab towards the background. This is currently missing, but if needed it should run in the background page because connection to the background is guaranteed (by the messenger’s own retries), it shouldn't need to retry.
• RemoteExecutionError was created in the source tab, but now it's generated and serialized in the background page. By the looks of it, none of the checks use instanceof to check this error. .name === 'RemoteExecutionError' works correctly.

Demo (opener)

Screen.Recording.1.mov

[fregante]

@twschiller this may be good to go but first it's probably best for you to do some additional tests (especially for the server and UiPath) and review the Concerns section above.

[twschiller]

Doesn't this change the return type? The old code destructured data to get the {data, error}? (Since IIRC the server will return 200 even on block error, since the request itself succeeded)

• Confirm onServer return type

[twschiller]

Oops, I somehow missed the executor diff

[fregante]

Yes I moved this into the background handler.

No worries, diffs aren't always immediately clear.

[fregante]

This is actually part of the Concerns section and it's something I'd like you to test with the server.

[twschiller]

I'm not the biggest fan of the word "request" here because it make me think "network request" and only the REQUEST_RUN_ON_SERVER message would make a request

Any reason to use request here vs. execute (or another word)

[fregante]

Yeah I suppose that they could all use RUN_IN_* and then keep RUN_BRICK for the final call. I was just trying to differentiate the forwarder from the final call, as mentioned in the first post.

[twschiller]

Confirm return type for execute on server

[twschiller]

Looks good, will make sure to test in more detail during QA/Rainforest tests.

Main comment is I'm not the biggest fan of the name "requestRun", but we can always revisit that later