fix: socket hang up error 422 issues in github publish #6563
Conversation
|
✔️ Deploy Preview for car-park-attendant-cleat-11576 ready! 🔨 Explore the source changes: e88dffd 🔍 Inspect the deploy log: https://app.netlify.com/sites/car-park-attendant-cleat-11576/deploys/61e733dfe3b9920007934812 😎 Browse the preview: https://deploy-preview-6563--car-park-attendant-cleat-11576.netlify.app |
🦋 Changeset detectedLatest commit: e88dffd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
this should in theory close #4940 |
baparham
changed the title
|
@mmaietta can you take a look at this? |
baparham
force-pushed
the
fix-error-422
branch
from
4645d02
to
e4caeca
Compare
baparham
force-pushed
the
fix-error-422
branch
from
e4caeca
to
ed6497b
Compare
| const desc = e.description | ||
| const descIncludesAlreadyExists = | ||
| (desc.includes("errors") && desc.includes("already_exists")) || (desc.errors && desc.errors.length >= 1 && desc.errors[0].code === "already_exists") | ||
| doesErrorMeanAlreadyExists = doesErrorMeanAlreadyExists && descIncludesAlreadyExists |
There was a problem hiding this comment.
We can just simplify the logic flow here to be less nested (instead of refactoring after)
private doesErrorMeanAlreadyExists(e: any) {
if (!e.description) {
return false
}
const desc = e.description
const descIncludesAlreadyExists = (desc.includes("errors") && desc.includes("already_exists")) || (desc.errors && desc.errors.length > 0 && desc.errors[0].code === "already_exists")
return e.statusCode === 422 && descIncludesAlreadyExists
}
There was a problem hiding this comment.
yeah, that might make more sense that way, a bit easier to follow at least
There was a problem hiding this comment.
I've fixed it up to be more like your suggestion
| if (e.description) { | ||
| const desc = e.description | ||
| const descIncludesAlreadyExists = | ||
| (desc.includes("errors") && desc.includes("already_exists")) || (desc.errors && desc.errors.length >= 1 && desc.errors[0].code === "already_exists") |
There was a problem hiding this comment.
I'm not sure I understand this:
desc.includes("errors") && desc.includes("already_exists")
Are we expecting a string or array for desc?
There was a problem hiding this comment.
the old code seemed to expect an object, (e.g. description = { ..., errors: [..., ..., ]}) but the response back from github is a string with some prose, and some JSON style notation that we really can't parse properly, so I support both ways, not really sure what to expect.
…heck into function
baparham
force-pushed
the
fix-error-422
branch
from
ed6497b
to
0f66b0f
Compare
|
Great work! Thank you for the contribution :) |
I split this up into two commits, one with the actual fix, then one refactor so you could actually understand what the if statement was looking for before calling
this.overwriteArtifact