fix: Fixed timeouts in case of slow XPI uploads to the signing server#213
Conversation
rpl
left a comment
rpl
left a comment
There was a problem hiding this comment.
This patch looks good, but I've a doubt about the signedStatusCheckTimeout value, which is described in more detail in the comment below.
| apiJwtExpiresIn=60 * 5, // 5 minutes | ||
| debugLogging=false, | ||
| signedStatusCheckInterval=1000, | ||
| signedStatusCheckTimeout=120000, // 2 minutes. |
There was a problem hiding this comment.
@kumar303 the signedStatusCheckTimeout is now lower then the apiJwtExpiresIn time, shouldn't we raise it a bit? (to be a bit longer than the new JWT expiration time), am I right or am I missing something?
or maybe it is better to compute the signedStatusCheckTimeout value from the apiJwtExpiresIn value if not explicitly set in the parameters, how it sounds to you?
(and we should also add some new tests about the combined usage of the two values)
There was a problem hiding this comment.
This setting is actually unrelated and I think it makes sense to keep it at 2 minutes. Here is a rough sketch of what happens:
- sign-addon uploads the file by making a POST or PUT to the API. This might take a while due to the size of the add-on and network connectivity.
- The code begins its
signedStatusCheckTimeouttimer. - The code makes a GET request to the API in a loop and this must finish within 2 minutes.
However, you just made me realize the HTTP request could timeout. Fixed, thanks!
@kumar303 that's great! r+ |
2 participants
The maximum allowed expiration time for JWT authentication has been increased in mozilla/addons-server#3686 to address a problem where slow file uploads delay the validation of the JWT (and could cause an expiration). This bumps the default expiration in the client and also lets the caller configure the value (if needed).