-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unclear failure mode with "undefined" version #59
Comments
hey, thanks for the repo. Yep, it seems that it was not able to determine the version. Does it happen often? |
I hadn't seen it before this afternoon, though I've now seen it twice. |
Hmm. Hard to tell what's going on atm. It is relying on the |
Are there any debug flags I could turn on in our CI which might yield more information? Edit: looks like not, based on reading the |
Yes, I also don't see it. Here is the method I would assume that is getting the latest version id: https://github.com/iterative/setup-dvc/blob/v1/src/utils.js#L35-L41 |
Hrm, there's no checking there of the HTTP status code. I'm not an expert on |
Looks like HTTP status codes are not considered exceptions and need explicit handling (via https://www.npmjs.com/package/node-fetch#handling-exceptions) |
Yep, we would be happy to accept a quick PR and release if you have time for this. |
This attempts to detect such errors, assuming that they appear with suitable HTTP status codes, and raises useful errors for them. Fixes iterative#59
This attempts to detect such errors, assuming that they appear with suitable HTTP status codes, and raises useful errors for them. Fixes iterative#59
FWIW, I don't think I've seen this since the spate of occurrences around the time I raised this issue. |
We've just seen this again. Any news on #60? I think that would at least help diagnose things. |
* Detect and handle errors fetching the latest version This attempts to detect such errors, assuming that they appear with suitable HTTP status codes, and raises useful errors for them. Fixes #59 * Make a trivial change to trigger workflows * Undo trivial change --------- Co-authored-by: Helio Machado <[email protected]>
@PeterJCLaw hey, sorry for the delay - we merged it. |
I've seen this issue occurring a few times in the past couple of weeks.
We've had this action working as-is for ages and haven't changed anything about it, and only just started seeing this issue pop up last week, but it looks the same as what @PeterJCLaw previously reported. |
Unable to reproduce; this works on my computer: ™ on: push
jobs:
test:
runs-on: ubuntu-latest
steps:
- uses: iterative/setup-dvc@v1
with:
version: latest
- run: dvc doctor |
@BotScutters, please create a new issue if your issue persists, and share your |
if you can do that - I think it makes sense. I doubt it can help with this issue, but at least it will be using a non-deprecated Node version. |
It mysteriously worked on fifth try of rerunning the action, so... even I can't reliably reproduce it. Seems like the root cause is likely upstream of you. I'll create a new ticket / investigate this further if/when it happens again. |
I'm not completely sure what failed, however
onea couplea few of our CI runsjustfailed on this action with:A retry of the same run shortly afterwards succeeded.
My guess is that the logic to determine the version number failed, however did so in a manner which wasn't immediately an error. Perhaps if the version number lookup fails it should error immediately?
The text was updated successfully, but these errors were encountered: