Skip to content

citgm-smoker-nobuild seems broken #1028

Closed

Description

@BethGriggs

https://ci.nodejs.org/job/citgm-smoker-nobuild/nodes=rhel8-x64/1548/console fails with:

$ curl -O 'https://nodejs.org/download/release/<!DOCTYPE html><html><head><title>Index of downloadrelease<title><meta name='\''viewport'\'' content='\''width=device-width, initial-scale=1.0'\'' ><meta charset='\''utf-8'\'' ><style type='\''textcss'\''>td { padding-right: 16px; text-align: right; font-family: monospace }td:nth-of-type(1) { text-align: left; overflow-wrap: anywhere }td:nth-of-type(3) { white-space: nowrap } th { text-align: left; } @media(prefers-color-scheme: dark) { body { color: white; background-color:#1c1b22; } a { color: #3391ff; } a:visited { color: #C63B65; } }<style><head><body><h1>Index of downloadrelease<h1><table>

I am guessing it's related to changes to what https://nodejs.org/download/release/ returns - I notice the webpage is now styled.

Activity

  1. richardlau commented on Nov 13, 2023

    @richardlau
    Member

    FWIW the job is running:

    cd $WORKSPACE && rm -rf *
    if [ -n "$DOWNLOAD_LOCATION" ]; then
    	DOWNLOAD_DIR="https://nodejs.org/download/$DOWNLOAD_LOCATION/"
    else
    	DOWNLOAD_DIR="https://nodejs.org/download/release/"
    fi
    LINK=`curl $DOWNLOAD_DIR | grep $NODE_VERSION | sort -t. -k 1,1n -k 2,2n -k 3,3n | tail -1 | cut -d\" -f 2 | tr -d /` 
    case $nodes in
      *-ppcle|*-ppc64le) OS=linux; ARCH=ppc64le; EXT=tar.gz;;
      ubuntu*|debian*|fedora*|centos*|rhel*-x64) OS=linux; ARCH=x64; EXT=tar.gz;;
      osx*) OS=darwin; ARCH=x64; EXT=tar.gz;;
      *-s390x) OS=linux; ARCH=s390x; EXT=tar.gz;;
      aix*) OS=aix; ARCH=ppc64; EXT=tar.gz;;
      win*) OS=win; ARCH=x64; EXT=zip;;
    esac
    curl -O "$DOWNLOAD_DIR$LINK/node-$LINK-$OS-$ARCH.$EXT"
    case $nodes in
      win*) unzip node-$LINK-$OS-$ARCH.$EXT ;;
      *) gzip -cd node-$LINK-$OS-$ARCH.$EXT | tar xf - ;;
    esac
    mv node-$LINK-$OS-$ARCH node
  2. targos commented on Nov 13, 2023

    @targos
    Member
  3. ovflowd commented on Nov 13, 2023

    @ovflowd
    Member

    Quick stupid question, but are really making a GET request with a huge html snippet in the URL?

  4. ovflowd commented on Nov 13, 2023

    @ovflowd
    Member

    Or what exactly is the issue here?

  5. flakey5 commented on Nov 13, 2023

    @flakey5
    Member

    Quick stupid question, but are really making a GET request with a huge html snippet in the URL?

    +1 to that and also is this relying on parsing whatever html is returned?

  6. ovflowd commented on Nov 13, 2023

    @ovflowd
    Member

    (Just trying to understand whay the script is trying to do and what is failing here)

  7. ovflowd commented on Nov 13, 2023

    @ovflowd
    Member

    Quick stupid question, but are really making a GET request with a huge html snippet in the URL?

    +1 to that and also is this relying on parsing whatever html is returned?

    The html looks like the index of that path, maybe something is wrong and accidentally it is trying to send the html, I have no idea. If someone can explain what this script is doing and what it is supposed to do

  8. targos commented on Nov 13, 2023

    @targos
    Member

    The script does this:

    curl https://nodejs.org/download/release/ | grep v20 | sort -t. -k 1,1n -k 2,2n -k 3,3n | tail -1 | cut -d\" -f 2 | tr -d /
    
  9. targos commented on Nov 13, 2023

    @targos
    Member

    You can compare with direct:

    curl https://direct.nodejs.org/download/release/ | grep v20 | sort -t. -k 1,1n -k 2,2n -k 3,3n | tail -1 | cut -d\" -f 2 | tr -d /
    
  10. ovflowd commented on Nov 13, 2023

    @ovflowd
    Member

    The script does this:

    
    curl https://nodejs.org/download/release/ | grep v20 | sort -t. -k 1,1n -k 2,2n -k 3,3n | tail -1 | cut -d\" -f 2 | tr -d /
    
    

    The script might need to be adjusted to accommodate the new html structure of our directory listing pages.

    Heh, @flakey5 I did say somewhere we were relying on the format of the directory listing k

  11. richardlau commented on Nov 13, 2023

    @richardlau
    Member

    I suspect the existing script is relying on line breaks in the html listings, but the new page has everything on a single line.

    FWIW https://ci.nodejs.org/job/node-test-node-addon-api-new/ does something similar and also failing.

  12. targos commented on Nov 13, 2023

    @targos
    Member

    It would be unfair (and a lot of time lost in total) to expect everyone who was parsing nginx output to change their scripts if there's a way to fix that in the new infra.

  13. richardlau commented on Nov 13, 2023

    @richardlau
    Member

    I'm confused -- I thought https://nodejs.org/download/release/ was still supposed to be served from the DO/Joyent servers and not R2 so shouldn't have changed?

  14. ovflowd commented on Nov 13, 2023

    @ovflowd
    Member

    It would be unfair (and a lot of time lost in total) to expect everyone who was parsing nginx output to change their scripts if there's a way to fix that in the new infra.

    Oh yeah, definitely agree with you here.

  15. targos commented on Nov 13, 2023

    @targos
    Member

    I'm confused -- I thought https://nodejs.org/download/release/ was still supposed to be served from the DO/Joyent servers and not R2 so shouldn't have changed?

    Only /dist is still served from DO/Joyent. Other routes are served from R2 since Nov 8.

  16. flakey5 commented on Nov 13, 2023

    @flakey5
    Member

    I suspect the existing script is relying on line breaks in the html listings, but the new page has everything on a single line.

    +1 I think this is what it's relying on. Formatted the html response and got it to return this
    image

    When using the test command @targos pointed out it returns just v20.9.0

  17. ovflowd commented on Nov 13, 2023

    @ovflowd
    Member

    I suspect the existing script is relying on line breaks in the html listings, but the new page has everything on a single line.

    +1 I think this is what it's relying on. Formatted the html response and got it to return this image

    When using the test command @targos pointed out it returns just v20.9.0

    Do we have any updates here? Or issue to keep track of this work on the worker side of things?

  18. flakey5 commented on Nov 13, 2023

    @flakey5
    Member

    Working on a fix on the worker side of things here nodejs/release-cloudflare-worker#65

  19. mhdawson commented on Nov 15, 2023

    @mhdawson
    Member

    @flakey5 good to hear you are working on it. Let me know when the fix is in place so that I can confirm it fixes the Node-api tests which were also broken (https://ci.nodejs.org/view/x%20-%20Abi%20stable%20module%20API/job/node-test-node-addon-api-new/).

  20. flakey5 commented on Nov 15, 2023

    @flakey5
    Member

    The jobs should be fixed now. We moved the worker off of /download until nodejs/release-cloudflare-worker#74 is resolved due to multiple issues that we've been seeing

  21. ovflowd commented on Nov 15, 2023

    @ovflowd
    Member

    Well, can we manually test that they will not break once we move them back? We just merged the new directory listing can we check that?

  22. flakey5 commented on Nov 15, 2023

    @flakey5
    Member

    Once nodejs/release-cloudflare-worker#76 lands this issue should be fixed on the worker's end

  23. flakey5 commented on Nov 23, 2023

    @flakey5
    Member

    Ran the command against the worker again and it works
    image

  24. targos commented on Dec 23, 2024

    @targos
    Member

    This specific issue is fixed. If something needs to be changed long-term, please open an issue and suggest what / how

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions