Skip to content

chore: no need to error out when s3 bulk payloads return 404#4263

Open
mnafees wants to merge 1 commit into
mainfrom
nafees/s3-404-no-err
Open

chore: no need to error out when s3 bulk payloads return 404#4263
mnafees wants to merge 1 commit into
mainfrom
nafees/s3-404-no-err

Conversation

@mnafees

@mnafees mnafees commented Jun 23, 2026

Copy link
Copy Markdown
Member

Description

Instead of treating expired and nvalid S3 keys as errors, let us only WRN for these for bulk payload processing.

Type of change

  • Chore (changes which are not directly related to any business logic)

What's Changed

  • Helper to check for S3 404 or NoSuckKey error

Checklist

Changes have been:

  • Tested (unit, integration, or manually with steps specified)
  • Linted and formatted
  • Documented (where applicable)

🤖 AI Disclosure
  • I acknowledge that an LLM was used in the creation of this Pull Request, in accordance with Hatchet's AI_POLICY.md.
  • Details: [e.g. generating tests, writing docs]

@mnafees mnafees requested a review from mrkaye97 June 23, 2026 14:08
@mnafees mnafees self-assigned this Jun 23, 2026
Copilot AI review requested due to automatic review settings June 23, 2026 14:08
@vercel

vercel Bot commented Jun 23, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hatchet-docs Ready Ready Preview, Comment Jun 23, 2026 2:09pm

Request Review

@github-actions github-actions Bot added the engine Related to the core Hatchet engine label Jun 23, 2026
@mrkaye97

Copy link
Copy Markdown
Contributor

We sure about this? If this happens, then the task we're about to trigger will have an empty input

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts dispatcher logging to reduce alert noise when bulk payload retrieval from S3 fails due to expected “not found” conditions (e.g., expired/cleaned-up payload keys).

Changes:

  • Adds isS3NotFoundErr helper to detect S3 “not found” errors (404 / NoSuchKey).
  • Downgrades bulk payload retrieval log level from ERROR to WARN when the failure is classified as an expected S3 “not found”.

Comment on lines +648 to +668
func isS3NotFoundErr(err error) bool {
var nsk *types.NoSuchKey
if errors.As(err, &nsk) {
return true
}

var respErr *smithyhttp.ResponseError
if errors.As(err, &respErr) && respErr.HTTPStatusCode() == 404 {
return true
}

var apiErr smithy.APIError
if errors.As(err, &apiErr) {
switch apiErr.ErrorCode() {
case "NoSuchKey", "NotFound":
return true
}
}

return false
}
gregfurman
gregfurman previously approved these changes Jun 23, 2026
@gregfurman gregfurman dismissed their stale review June 23, 2026 21:23

Nvm. I saw Matt's comment. Will await feedback before approving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine Related to the core Hatchet engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants