fix: Resolve renderer sendFeedback only after ingest accepts it

[satoren]

Summary

• sendFeedback is documented to resolve only when Sentry accepted the feedback, but the renderer transport always returned 200 before the envelope reached main. Protocol fetch was not awaited, and failures were discarded.
• Wait for the protocol or IPC handoff, then return the main-process ingest status. Handoff failure, 4xx/5xx, enabled: false, and an offline-queue write are not 2xx, so sendFeedback rejects. A queued envelope may still be retried from disk later (Is it okay than sentry/electron transport drops all info from response and sentry/core base-transport tries to check it? #942).

Closes #1428

Test plan

• yarn test renderer-transport.test.ts
• ELECTRON_VERSION=25.9.8 yarn e2e -t 'User Feedback' including the protocol fallback, a 64KB attachment, and window.close() immediately after sendFeedback resolves
• Confirm a renderer sendFeedback rejects when the main SDK is enabled: false or ingest returns 413
• Confirm a network failure queues the envelope and rejects sendFeedback instead of resolving

Made with Cursor

[sentry]

Bug: When multiple feedbacks are sent concurrently, a flawed check feedbackDropWaiters.size !== 1 prevents drop notifications, causing the sendFeedback promise to hang for 60 seconds.
_{Severity: HIGH}

Suggested Fix

The check feedbackDropWaiters.size !== 1 is flawed and should be removed. The logic should be updated to correctly notify the appropriate waiter when a feedback event is dropped, even when multiple feedbacks are being processed concurrently. This might involve iterating through the waiters and finding the correct one to call, or redesigning how waiters are managed.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/main/ipc.ts#L110-L112

Potential issue: A module-level `Set`, `feedbackDropWaiters`, is shared across all
concurrent feedback submissions. When multiple feedbacks are in-flight, its size becomes
greater than one. If one of these feedbacks is dropped (e.g., by `beforeSend`), the
check `feedbackDropWaiters.size !== 1` in the patched `recordDroppedEvent` handler
evaluates to true. This incorrectly prevents the `onDrop` callback from being invoked.
As a result, the corresponding `sendFeedback` promise never settles and hangs for its
full 60-second timeout instead of rejecting immediately, severely degrading user
experience in applications with multiple renderer windows.

[timfish]

Thanks for digging into this. Before we go further I'd like to understand what actually goes wrong for users.

As far as I can tell, the only case where feedback really gets lost is protocol mode: if the window closes right after sendFeedback resolves, the fetch can get aborted. Is that what you ran into? If so, we can fix it by waiting until the main process has the envelope, which is a much smaller change.

I'm less sure about waiting for the response from Sentry. The main process already retries and keeps an offline queue, so when the network is down the feedback still gets sent later. With this PR, sendFeedback would reject in that case. The app would show an error, the user would likely submit again, and we'd get duplicates. The other failures (413, 429, enabled: false) are rare for feedback. Also, a 429 gets passed to the renderer transport as is, and that makes the renderer drop all its events for 60 seconds.

[satoren]

Thank you for looking into this.

To answer your question about what goes wrong for users: our customers were experiencing an issue where they submitted feedback, but we never received it. This was likely due to the requests being blocked by their corporate proxies. In these cases, the users believed their feedback was successfully sent (because the app UI indicated success), but it never actually reached us.

Regarding the API design, as stated in the documentation, captureFeedback is intended for cases where the result doesn't need to be awaited. However, the docs explicitly state that sendFeedback "resolves only when the feedback was successfully sent to Sentry." The current renderer implementation resolves immediately without confirming delivery, which violates this documented contract.

[timfish]

Thanks again for the report and for the PR, it made the problem very clear. I've opened #1433 with a different approach and I'd like to explain why rather than ask for a rework here.

A few things in this PR would cause new problems:

• A 429 from Sentry is passed to the renderer without its headers. The renderer transport then treats it as "rate limit everything for 60 seconds", so one rate-limited feedback drops every error, span and replay from that renderer for a minute.
• NOT_DELIVERED is { statusCode: 0 } and it's returned for every envelope type when the handoff fails or main has enabled: false. Replay treats any numeric non-2xx status as a fatal send error and stops recording, so a single failed handoff would kill replay for that window. Core uses {} for "no result" and replay tolerates that.
• Drop detection replaces client.recordDroppedEvent for the life of the process and only works while exactly one feedback is in flight. With two, if one is dropped, both wait for the 60 second timeout.
• The envelope channel is changed for every envelope (invoke, awaited protocol fetch, status decoding), even though only feedback needs it. That's a lot of surface for a change that only sendFeedback reads.
• The ipcMain.on handler is kept next to the new ipcMain.handle on the same channel, but nothing calls it any more.

#1433 keeps the envelope channel exactly as it is and adds a separate feedback channel. Main prepares the feedback event with prepareEvent and sends it with client.sendEnvelope, which returns the transport result directly. That removes the afterSendEvent matching, the timeout and the drop detection: beforeSend and sampleRate don't apply to feedback events, so there's nothing captureEvent would do that we'd miss. The result is passed to the renderer with headers, so a rate limit only affects feedback.

The behaviour for your case is the same as here: sendFeedback resolves on a 2xx and rejects when the send failed, was queued offline, or main is disabled.

The enabled: false fix for spans and replays that this PR also included landed separately in #1432.

I'll close this in favour of #1433. Thanks for pushing on it.