Skip to content

fix: Check the error from constructing the event poll request - #67

Merged
keelerm84 merged 1 commit into
mainfrom
mk/SDK-2822/poll-request-error
Aug 31, 2026
Merged

fix: Check the error from constructing the event poll request#67
keelerm84 merged 1 commit into
mainfrom
mk/SDK-2822/poll-request-error

Conversation

@keelerm84

@keelerm84 keelerm84 commented Aug 31, 2026

Copy link
Copy Markdown
Member

Summary

eventLoop discarded the error from http.NewRequest and set a header on the
returned request immediately. On a construction failure that request is nil, so the
daemon panicked on a nil pointer dereference instead of reporting the problem.
featureLoop already checks the identical call, so the two loops now agree.

The loop returns the error rather than logging it and polling again. pollURI is
computed once from SALESFORCE_URL before the loop starts, so a URL that
url.Parse rejects fails identically on every iteration -- retrying would spin
forever with no way to make progress, and would bury an operator misconfiguration
in a log line.

The new test drives eventLoop with a SALESFORCE_URL holding a control
character, which is what makes url.Parse fail. It recovers a panic inside the
loop goroutine, so the previous behavior surfaces as one test failure rather than
killing the test binary. Verified to bite: with the check removed the test fails
with invalid memory address or nil pointer dereference.


Note

Overview
eventLoop now checks the error from http.NewRequest before setting headers on the poll request, matching featureLoop. A bad SALESFORCE_URL (e.g. one url.Parse rejects) used to yield a nil request and crash the daemon on header setup; the loop now returns that error instead of retrying, because pollURI is fixed for the process lifetime so retries would only spin.

A new test drives eventLoop with an invalid Salesforce base URL, asserts no panic, expects the same construction error as http.NewRequest, and times out if the loop keeps polling.

Reviewed by Cursor Bugbot for commit 227ea39. Bugbot is set up for automated code reviews on this repo. Configure here.

eventLoop discarded the error from http.NewRequest and set a header on the
returned request immediately. On a construction failure that request is nil, so
the daemon panicked on a nil pointer dereference instead of reporting the
problem.

The loop returns the error rather than logging it and polling again. pollURI is
computed once from SALESFORCE_URL before the loop starts, so a URL that
url.Parse rejects fails identically on every iteration; retrying would spin
forever with no way to make progress, and would hide an operator
misconfiguration behind a log line. featureLoop already returns from the
identical call, so this also makes the two loops agree.

err is reassigned a few lines down by the requestWithOauth call, which is in the
same block, so the new check reads the same variable rather than shadowing it
and nothing goes unused.

The test drives eventLoop with a SALESFORCE_URL holding a control character,
which is what makes url.Parse fail. It recovers a panic in the loop's goroutine
so the old behavior fails one test instead of killing the test binary.
@keelerm84
keelerm84 merged commit 7e9a41d into main Aug 31, 2026
6 checks passed
@keelerm84
keelerm84 deleted the mk/SDK-2822/poll-request-error branch August 31, 2026 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants