isRetryable retries permanent server-side failures (quota, read-only filesystem) with backoff

[eiserv]

What

isRetryable in internal/uploader/retry.go treats everything it does not recognise as worth retrying:

func isRetryable(err error) bool {
	switch {
	case errors.Is(err, context.Canceled), errors.Is(err, context.DeadlineExceeded):
		return false
	case errors.Is(err, os.ErrPermission), errors.Is(err, os.ErrNotExist):
		return false
	default:
		return true
	}
}

pkg/sftp maps SSH_FX_PERMISSION_DENIED to os.ErrPermission and SSH_FX_NO_SUCH_FILE to os.ErrNotExist, so those two are caught. Every other server-side status code falls through to true.

Why it matters

Several genuinely permanent server-side conditions arrive as SSH_FX_FAILURE (or another unmapped code) rather than as one of the two recognised errors:

• disk quota exceeded
• no space left on the target filesystem
• a read-only filesystem or a read-only mount point
• path or filename length limits
• server-side policy rejections (a file type or extension the server refuses)

None of these get better by waiting. Today each such file is attempted retries + 1 times with exponential backoff (1 s, then 2 s at the default retries: 2), and, because isConnError will not match them, without a reconnect either. The failure is guaranteed from the first attempt.

The cost is not just the delay. Consider a deploy that fills the account's quota partway through: every remaining file now fails three times with backoff. With a few thousand files still to go, a run that should fail in seconds with "quota exceeded" instead spends a long time doing it, and the log fills with retry warnings that bury the one line explaining what actually went wrong. On a paid runner that is billed minutes spent on a doomed deploy.

Worth noting the default is defensible in isolation: retrying an unknown error is the safer bias for transient network conditions, and misclassifying a transient error as permanent is worse than the reverse. The problem is that the well-known permanent codes are lumped in with the unknowns.

Suggested direction

Inspect the SFTP status code before falling through to the default:

var se *sftp.StatusError
if errors.As(err, &se) {
    switch se.FxCode() {
    case sftp.ErrSSHFxPermissionDenied,
         sftp.ErrSSHFxNoSuchFile,
         sftp.ErrSSHFxOpUnsupported,
         sftp.ErrSSHFxNoSuchPath:
        return false
    }
}

Keep default: true. The point is to name the codes that are known-permanent, not to invert the bias.

Open questions:

• SSH_FX_FAILURE is the hard one. It is the catch-all that quota and read-only-filesystem errors usually arrive as, and it is also what some servers return for genuinely transient problems. Classifying it as permanent would be wrong. Matching on the human-readable message text ("quota", "no space") is fragile and probably not worth it; leaving SSH_FX_FAILURE retryable is the safe call.
• Better messages matter more than the retry count. Even where retrying stays, the final error should surface the server's own status text prominently, so "quota exceeded" is what the user reads rather than the third retry warning.
• Whether the same classification should apply to session.do's ops, which use isConnError rather than isRetryable. Different question, but worth a look in the same pass.