fix: show contextual artifact unavailability message in preview dialog#2181
Open
morgan-wowk wants to merge 1 commit intomasterfrom
Open
fix: show contextual artifact unavailability message in preview dialog#2181morgan-wowk wants to merge 1 commit intomasterfrom
morgan-wowk wants to merge 1 commit intomasterfrom
Conversation
🎩 PreviewA preview build has been created at: |
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
morgan-wowk
force-pushed
the
artifact-retention-msgs
branch
9 times, most recently
from
d6fbf9f to
5fe2f3f
Compare
camielvs
reviewed
Apr 30, 2026
Comment on lines
+182
to
+187
| <ArtifactRetentionNotice | ||
| title="Failed to load artifact" | ||
| variant="error" | ||
| preamble={`An unexpected error occurred${statusDetail}.`} | ||
| specific | ||
| /> |
Collaborator
There was a problem hiding this comment.
Semantics here are a little confusing: given you're using this new component for both a Retention Notice and general errors you may want to consider a bit of renaming and/or splitting responsibilities between multiple components.
Author
There was a problem hiding this comment.
It is split into multiple components per your suggestion.
morgan-wowk
force-pushed
the
artifact-retention-msgs
branch
from
5fe2f3f to
8c77875
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Introduces a typed
ArtifactFetchErrorclass that carries HTTPstatusandstatusText, replacing genericErrorthrows in bothgetArtifactSignedUrlanduseArtifactFetch. This allows the artifact visualizer to distinguish between 404 (not found) and other failure states and render appropriate inline notices.Adds a new
ArtifactRetentionNoticecomponent that displays a configurable warning or errorInfoBoxwith a message driven by theVITE_ARTIFACT_RETENTION_DAYSenvironment variable. When the variable is set, the message includes the specific number of days; otherwise it falls back to a generic expiry message.The existing hardcoded 30-day retention warning in
IOSectionis replaced with this new component, now respectingVITE_ARTIFACT_RETENTION_DAYSinstead of a fixed threshold. The artifact visualizer'sSuspenseWrappernow uses anerrorFallbackthat renders a retention notice on 404 and an error-variant notice for all other failures.Related Issue and Pull requests
Type of Change
Checklist
Before screenshots
After screenshots
Test Instructions
ArtifactRetentionNoticewarning appears in the IO section.VITE_ARTIFACT_RETENTION_DAYSto a specific value and verify the expiry message includes that number of days.VITE_ARTIFACT_RETENTION_DAYSand verify the generic expiry message is shown instead.Additional Comments
The
Object.setPrototypeOfcall inArtifactFetchErrorensuresinstanceofchecks work correctly when targeting ES5 in TypeScript.