Fix Deno check precluding esm.sh to start #327
Conversation
server/nodejs.go
Outdated
| @@ -633,7 +633,7 @@ func toTypesPackageName(pkgName string) string { | |||
| } | |||
|
|
|||
| func getDenoStdVersion() (version string, err error) { | |||
| resp, err := httpClient.Get("https://cdn.deno.land/std/meta/versions.json") | |||
| resp, err := http.Get("https://cdn.deno.land/std/meta/versions.json") | |||
There was a problem hiding this comment.
Is this because of the HTTP_PROXY issue you mentioned? If so, want to try to add Proxy field to the Transport config, to see if it works for you? It's essentially dropping in this line:
Proxy: http.ProxyFromEnvironment,right below this line
Line 34 in 46940b3
There was a problem hiding this comment.
@jimisaacs done, it seems to work. Thanks, go is not my usual language :)
There was a problem hiding this comment.
Mine neither to be honest, I approved and am running the workflow, but I think @ije should give this change a look. I don't think that setting should have any overhead given that it's the default of go, but I wouldn't know at the scale @ije is operating this service.
FYI, the check failure looks unrelated to your change.
| @@ -143,7 +143,7 @@ func Serve(efs EmbedFS) { | |||
|
|
|||
| denoStdVersion, err = getDenoStdVersion() | |||
| if err != nil { | |||
| log.Fatalf("getDenoStdVersion: %v", err) | |||
| log.Warnf("getDenoStdVersion: %v", err) | |||
There was a problem hiding this comment.
If you are not comfortable with my suggestion, please add a TODO / FIXME here to look at it later.
In build.go, there are places denoStdVersioin is used (lines 487, 706, 713).
There, it needs to check if denoStdVersioin is set, and if not build a string without it.
Example, this is what it would look like with it set: https://deno.land/std@0.139.0/archive/tar.ts
This is what it would look like without it set: https://deno.land/std/archive/tar.ts
The URL without it still works without the version with a redirect, so this is a valid change, but really only with the string update because that @ in the URL is invalid without a version there.
| @@ -143,7 +143,7 @@ func Serve(efs EmbedFS) { | |||
|
|
|||
| denoStdVersion, err = getDenoStdVersion() | |||
| if err != nil { | |||
| log.Fatalf("getDenoStdVersion: %v", err) | |||
| log.Warnf("getDenoStdVersion: %v", err) | |||
| } | |||
| log.Debugf("https://deno.land/std@%s found", denoStdVersion) | |||
There was a problem hiding this comment.
Can you move this to an else, since it's not found ;)
This PR relates to #309
It essentially:
Http.Getfor it to honour env-defined http proxy URLs