🎨 Palette: Smarter retries and better error feedback

[google-labs-jules]

Implemented a UX improvement for the Unipept CLI error handling.

• Smarter Retry Logic: The CLI no longer blindly retries 5 times for 4xx client errors (like 404 Not Found). It now fails immediately for these permanent errors, while still retrying for network issues, 5xx server errors, and 429 Too Many Requests.
• Improved Error Feedback: The error message displayed in the console now includes the actual error text (e.g., "404 Not Found") instead of just pointing to a log file.
• Verification: Added tests/commands/unipept/retry_logic.test.ts to fully test the new retry logic.

---

PR created automatically by Jules for task 7518209817329185754 started by @bmesuere

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[pverscha]

I checked how these status codes are handled on our HAProxy setup, and based on that the retry logic in this PR looks correct to me.

One important note is that we currently do not have “real” rate limiting enabled. I experimented with it briefly in the past, but it turned out to be more annoying than helpful, so it was disabled again. That said, there is a maximum number of parallel connections allowed between the load balancer and the backend API servers. If that limit were ever exceeded across all servers, HAProxy could return a 429, but this situation should be very rare in practice.

In normal failure scenarios, 503 (Service Unavailable) is returned when all backend servers are down. This can occasionally happen even when we are not yet at the maximum connection limit, for example if all servers are overloaded and stop responding to health checks in time. In that case, retrying the request after a short delay (as implemented in this PR) is indeed the correct and useful behavior.

Overall, the retry conditions used here (429 and 5xx responses) are well aligned with how our infrastructure behaves, so I’m happy to approve this PR.

[Copilot]

Pull request overview

This PR implements smarter retry logic and improved error feedback for the Unipept CLI. Instead of blindly retrying all errors 5 times, the CLI now distinguishes between permanent client errors (4xx) that should fail immediately and transient errors (5xx, network issues, 429) that warrant retries.

Key changes:

• Implemented selective retry logic that immediately fails on 4xx errors (except 429) while retrying on 5xx, network errors, and rate limiting
• Enhanced error messages to display the actual error text in the console instead of just pointing to a log file
• Added comprehensive test coverage for the new retry behavior

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
lib/commands/unipept/unipept_subcommand.ts	Added smart retry logic to distinguish between permanent and transient errors; improved error feedback to display error messages directly to users; fixed missing await on mkdir
tests/commands/unipept/retry_logic.test.ts	New test file with comprehensive coverage of retry scenarios including 500 errors, network errors, 404 errors, 400 errors, and 429 rate limiting

Comments suppressed due to low confidence (1)

lib/commands/unipept/unipept_subcommand.ts:80

• Avoid automated semicolon insertion (91% of all statements in the enclosing function have an explicit semicolon).

      })

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The status code parsing logic is fragile. If the error string format changes or doesn't match the expected pattern (status code followed by space), parseInt will return NaN. While NaN comparisons will fail safely (resulting in retry), it would be more robust to validate the parsed status before using it, or use a regex to extract the status code.

Suggested change

	const status = parseInt(error.split(' ')[0]);
	if (status >= 400 && status < 500 && status !== 429) {
	shouldRetry = false;
	const match = /^(\d{3})\b/.exec(error);
	if (match) {
	const status = parseInt(match[1], 10);
	if (Number.isFinite(status) && status >= 400 && status < 500 && status !== 429) {
	shouldRetry = false;
	}

[Copilot]

The comment on line 234 is slightly misleading. The code checks the error message format, not just errors "from the reject above". It also handles the case where the error is not a string (e.g., network errors), which are implicitly retryable. Consider clarifying this comment to explain both cases: string errors (HTTP status codes) and non-string errors (network failures).

Suggested change

	// check if we should stop retrying based on error message (from the reject above)
	// For string errors (e.g. "status statusText" from the reject above), stop retrying
	// on non-429 4xx responses. For non-string errors (e.g. network failures thrown by
	// fetch), keep shouldRetry as-is so they remain retryable.