Repository navigation
fix: check whether AMO attached the source after a gateway error - #3912
Open
itsnjstyle27 wants to merge 1 commit into
Open
itsnjstyle27 wants to merge 1 commit into
itsnjstyle27 wants to merge 1 commit into
Conversation
When the source upload gets a 502, 503 or 504, poll the version details for up to a minute and only continue if AMO reports a source for the version. Otherwise fail with the same error as before.
itsnjstyle27
marked this pull request as ready for review
October 23, 2026 10:32
Author
|
Hi maintainers! Just checking in on this PR whenever you get a chance. I’d appreciate your feedback and am happy to make any changes needed. Thanks for your time! |
Member
|
Thanks for your PR. The team has reduced review capacity right now, and is unable to provide consistent reviews for the next two weeks. |
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
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.
Relates to #3807
When AMO answers the source upload with a 502, 503 or 504,
web-ext signfails right away. Sometimes AMO finishes the upload anyway, after the proxy in front of it has already given up, which is what the report in #3807 describes.In #3808 I tried treating those responses as a success, and @rpl explained why that's wrong: web-ext can't assume the upload went through. This PR takes a different approach. Instead of assuming either way, web-ext now asks AMO.
After a 502, 503 or 504 on the source upload, web-ext polls the version details for up to a minute. It's the same endpoint
waitForApprovalalready uses. It checks thesourcefield, which AMO only fills in once a source file is attached to the version.Nothing changes for other errors. A 500, a 4xx, or a gateway error on a patch that isn't a source upload all fail exactly as they do now.
Tests: I added unit tests for these cases:
The four tests where the upload should continue fail without this change. ESLint and the full unit suite pass. One file watcher test failed once when I ran everything together on Windows, but it passes on its own both with and without this change.
If you'd still rather keep web-ext strict here, that's fine and I'm happy to close this.