Fix request timeouts under concurrent load

[nDmitry]

Summary by CodeRabbit

• Performance Improvements

  • Image size detection now uses HEAD requests instead of full downloads, reducing bandwidth.

• Bug Fixes

  • Extended server request timeout from 30s to 300s to accommodate longer operations.
  • Improved logging for response write failures to aid troubleshooting.

• Chores

  • Removed unused configuration/constants and eliminated unnecessary file I/O.

[coderabbitai]

Walkthrough

This PR increases the server write timeout, changes response error logging in the Telegram handler, optimizes image size retrieval by using HTTP HEAD (removing temp file I/O), and removes an unused tmpPath constant.

Changes

Cohort / File(s)	Summary
Server Configuration internal/api/rest/server.go	Increased WriteTimeout from 30s to 300s to allow longer request processing (including semaphore wait and scraping).
Error Handling internal/api/rest/telegram.go	On write errors in serveContent, replaced handleBadErrorResponse call with h.logger.Error(...) logging; no additional response is written.
Image Processing internal/feed/extract.go	Switched image size retrieval from full GET to HEAD; removed temporary file creation and body streaming; now returns ContentLength when available and updates error text.
Code Cleanup internal/feed/scraper.go	Removed unused top-level constant tmpPath = "/tmp".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I hopped through timeouts, stretched them wide and long,
I sniffed the headers—no full-body song,
I logged a quiet whisper where a shout once stood,
I nibbled old constants till the code felt good. 🥕✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly addresses the main change: extending the server WriteTimeout from 30s to 300s to handle concurrent load scenarios more effectively.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

internal/api/rest/server.go (1)

48-48: 300s WriteTimeout is a blunt fix — consider a more targeted approach.

A 5-minute WriteTimeout means any connection can hold server resources for that duration, which increases exposure to slow-write attacks and resource exhaustion under load. Since the bottleneck is semaphore wait + scraping, consider using per-handler context.WithTimeout with a tighter server-level WriteTimeout (e.g., keep it at 60–90s and give the handler its own deadline). Alternatively, if concurrency control (semaphore) is the reason requests queue up, tuning the semaphore or adding backpressure/503 Service Unavailable responses for excess load would be more surgical.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/feed/extract.go (1)

329-349: ⚠️ Potential issue | 🟡 Minor

Good optimization switching to HEAD requests. One gap: the response status code is not checked before trusting ContentLength.

A non-2xx response (e.g., 403, 404, 500) could still carry a Content-Length header representing an error page body, not the image. Consider adding a status check:

Proposed fix

 	defer res.Body.Close()
 
-	if res.ContentLength > 0 {
+	if res.StatusCode >= 200 && res.StatusCode < 300 && res.ContentLength > 0 {
 		return res.ContentLength
 	}

🧹 Nitpick comments (1)

internal/api/rest/server.go (1)

48-48: Consider whether 300s WriteTimeout is appropriate for your deployment.

A 5-minute write timeout means each connection can be held open for that duration. While the comment explains the rationale (semaphore wait + scraping), this significantly increases the server's exposure to slow-client resource exhaustion. If you're behind a reverse proxy (e.g., nginx, Cloudflare), ensure it enforces its own tighter upstream timeout so that stalled connections don't accumulate at the Go server level.

If only a subset of endpoints require long processing, a per-request http.TimeoutHandler wrapper on those routes would let you keep a shorter global WriteTimeout.