[o11y] Do error checking in R2 earlier

[fhanau]

With #5685, we managed to reduce the volume of "destructed WorkerTracer" warnings by >80% by eliminating it in the case of duplicate alarm events. Another case where this happens are R2 API calls where we create a new WorkerInterface for the R2 call before completing error checking, as seen in the r2-test itself. We can easily avoid this by moving getHttpClient() calls behind error checks.
While this PR is not aiming for completion I also cleaned up a call in web-socket.c++ that may be susceptible to the same issue.

[fhanau]

@mar-cf: Re. tests: This is already being tested in //src/workerd/api/tests:r2-test – convince yourself that the warning is present on main but disappears after this change. Due to the number of different functions where this can happen, I can't write a regression test for each affected API.
Note that this PR is not aiming to exhaustively cover all cases where the issue appears – I merely noticed that on the edge we frequently get this warning for R2-related code, so this should reduce the error volume further. That being said the warning is of limited priority at this point since the volume is quite low and it merely indicates a bit of waste happening (we create a WorkerInterface and associated WorkerTracer that ends up unused due to an error we didn't check for yet). This PR achieves incremental progress, lmk if you think that isn't the right approach here.

[codspeed-hq]

CodSpeed Performance Report

Merging #5756 will not alter performance

_{Comparing felix/122325-stw-unused-tracer (923ab9f) with main (5b5dcee)}

Summary

✅ 140 untouched
⏩ 38 skipped¹

Footnotes

1. 38 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[danlapid]

Discussed privately and while this might improve things in local dev and so there's no reason not to run it - it won't actually sort out the prod errors because of some added layers in the prod environment between the user worker and R2's worker.

[mar-cf]

@mar-cf: Re. tests: This is already being tested in //src/workerd/api/tests:r2-test – convince yourself that the warning is present on main but disappears after this change.

I can see it fire off the base here, but on main it no longer fires for me.

Due to the number of different functions where this can happen, I can't write a regression test for each affected API.

Let's upgrade the error to a test assertion.

Given I can't see the effect in main and Dan's concerns about the additional prod layers posing a problem, I'm hesitant to believe this does anything. I'm not opposed to anything in here, so I'll stamp anyway if you feel like it's worth to have this. I think addressing Dan's concern is time better spent.

[fhanau]

I can see it fire off the base here, but on main it no longer fires for me.
Yes, that is since #5790 has been merged and the issues being addressed here result in worker stages never being delivered, so it is also being covered by that.

Given I can't see the effect in main and Dan's concerns about the additional prod layers posing a problem, I'm hesitant to believe this does anything. I'm not opposed to anything in here, so I'll stamp anyway if you feel like it's worth to have this. I think addressing Dan's concern is time better spent.

I'm not sure if there was any concern being raised per se – he just noted that this would not silence the warning in prod since R2 is implemented differently there. As you said this no longer silences any warnings in workerd, but it still allows us to avoid some superfluous memory allocations since in the error case we no longer have to construct a HttpClient and associated structures, plus a WorkerInterface/WorkerTracer within workerd – the impact is smaller than anticipated, which I will note in the updated commit description. It's still good practice to only do expensive operations after conducting error checking.