Uploading new avatar crashes forum?


  • sockdevs

    I'm not sure what's up right now but it appears that uploading a new avatar crashes the forum, as shown by servercooties.com

    0_1462361854068_upload-054cb0ab-1da1-408d-ad23-4b1f6213b4ff

    each one of those spikes corresponds to an attempt by me to upload a new avatar which causes a 502 response from /api/user/accalia/uploadpicture, then the websocket 502s and reconnects causing the forum updated toaster.....

    @ben_lubar ...... halp?



  • @accalia Try uploading it here in the thread. Avatars get some special treatment.


  • sockdevs

    @boomzilla 0_1462362071634_Avatar0123.png

    uploads here just fine....



  • @accalia Hmm...Locally, I get an error whenever I upload an avatar. The logs show something like:

    4/5 07:44 [16163] - error: /api/user/boomzilla/uploadpicture
     Error: ENOENT: no such file or directory, rename '/tmp/9-3S1sH3gbOaTuP6VDpz_duD.png.png' -> '/tmp/9-3S1sH3gbOaTuP6VDpz_duD.png'
        at Error (native)
    
    4/5 07:45 [16163] - error: /api/user/boomzilla/uploadpicture
     Error: ENOENT: no such file or directory, rename '/tmp/5jMOajFTijRaQAr4qvY9KlvY.jpg.png' -> '/tmp/5jMOajFTijRaQAr4qvY9KlvY.jpg'
        at Error (native)
    

    (running the head of nodebb master and the head of master at BenLubar/nodebb-plugin-imagemagick)





  • Sounds like we need an avatar CDN...


  • sockdevs

    @RaceProUK huh.... so the toaster went away, but the error's still there....

    well then.....


  • Discourse touched me in a no-no place

    Repro. Or at least the 502 aspect.

    0_1462363640326_Screenshot from 2016-05-04 13:06:41.png

    0_1462363674783_Screenshot from 2016-05-04 13:07:33.png

    And pasting from clipboard broken again it seems.

    One I was trying:

    0_1462363694970_hat_monicle.png


  • Winner of the 2016 Presidential Election

    Does this testing have anything to do with the constant "the forum was just updated" toasters?



  • The default nodeBB letter avatars are just a letter on a coloured background in HTML? Not an image created in 197 different sizes? I thought that was UNPOSSIBLE DOING_IT_WRONG ERR_NO_CDN_FOUND?!
    <div component="user/picture" data-uid="16" class="user-icon" style="background-color: #9c27b0;" data-original-title="" title="">P</div>



    Filed under: :doing_it_wrong:



  • 0_1462364614406_upload-b7bf7c5e-f9ca-4063-89e6-c78653b8f8c1

    @PJH said in Uploading new avatar crashes forum?:

    And pasting from clipboard broken again it seems.

    Works for me. You're not using FF are you?



  • @loopback0 said in Uploading new avatar crashes forum?:

    The default nodeBB letter avatars are just a letter on a coloured background in HTML? Not an image created in 197 different sizes? I thought that was UNPOSSIBLE DOING_IT_WRONG ERR_NO_CDN_FOUND?!

    They're probably not quite pixel perfect centered or something, which is absolutely unacceptable.


  • sockdevs

    @boomzilla said in Uploading new avatar crashes forum?:

    They're probably not quite pixel perfect centered or something

    /shrug close enough for guvmt work.



  • @accalia BIKESHED SAYS NO


  • sockdevs

    @boomzilla the bikeshed speaks? are you sure that's not just @end hiding behind the bikeshed and making his voice sound all weird so you don't recognize him and think it's the actual bikeshed speaking?


  • Discourse touched me in a no-no place

    @boomzilla said in Uploading new avatar crashes forum?:

    You're not using FF are you?

    Ah. That. Which browser I'm on tends to depend on which computer I'm using, and most of them are Chrome; this one's FF however.



  • @PJH You should be able to upload your balls now if you want to.


  • Discourse touched me in a no-no place

    @boomzilla How long before they're broken again?...



  • @PJH That's a question for @ben_lubar. See also.


  • Winner of the 2016 Presidential Election

    @boomzilla said in Uploading new avatar crashes forum?:

    @PJH You should be able to upload your balls now if you want to.

    @PJH said in Uploading new avatar crashes forum?:

    @boomzilla How long before they're broken again?...

    Get a room you two!


  • sockdevs

    @boomzilla said in Uploading new avatar crashes forum?:

    See also.

    403 ACCESS DENIED



  • 4/5 12:09 [2537] - error: TypeError: callback is not a function
        at /usr/src/app/src/user/data.js:17:4
        at /usr/src/app/src/user/data.js:23:4
        at /usr/src/app/src/plugins/hooks.js:110:4
        at /usr/src/app/node_modules/async/lib/async.js:380:13
        at /usr/src/app/node_modules/async/lib/async.js:52:16
        at /usr/src/app/node_modules/async/lib/async.js:269:32
        at /usr/src/app/node_modules/async/lib/async.js:44:16
        at /usr/src/app/node_modules/async/lib/async.js:377:17
        at Object.plugin.onForceEnabled [as method] (/usr/src/app/node_modules/nodebb-plugin-gravatar/library.js:114:3)
        at /usr/src/app/src/plugins/hooks.js:103:12
        at /usr/src/app/node_modules/async/lib/async.js:375:13
        at iterate (/usr/src/app/node_modules/async/lib/async.js:262:13)
        at Object.async.forEachOfSeries.async.eachOfSeries (/usr/src/app/node_modules/async/lib/async.js:281:9)
        at Object.async.inject.async.foldl.async.reduce (/usr/src/app/node_modules/async/lib/async.js:374:15)
        at fireFilterHook (/usr/src/app/src/plugins/hooks.js:95:9)
        at Object.Plugins.fireHook (/usr/src/app/src/plugins/hooks.js:76:5)
        at modifyUserData (/usr/src/app/src/user/data.js:131:11)
        at /usr/src/app/src/user/data.js:58:4
        at /usr/src/app/src/database/mongo/hash.js:136:4
        at handleCallback (/usr/src/app/node_modules/mongodb/lib/utils.js:96:12)
        at /usr/src/app/node_modules/mongodb/lib/cursor.js:848:16
        at handleCallback (/usr/src/app/node_modules/mongodb/node_modules/mongodb-core/lib/cursor.js:154:5)
        at setCursorDeadAndNotified (/usr/src/app/node_modules/mongodb/node_modules/mongodb-core/lib/cursor.js:463:3)
        at nextFunction (/usr/src/app/node_modules/mongodb/node_modules/mongodb-core/lib/cursor.js:644:7)
        at Cursor.next [as _next] (/usr/src/app/node_modules/mongodb/node_modules/mongodb-core/lib/cursor.js:685:3)
        at fetchDocs (/usr/src/app/node_modules/mongodb/lib/cursor.js:844:10)
        at /usr/src/app/node_modules/mongodb/lib/cursor.js:867:7
        at handleCallback (/usr/src/app/node_modules/mongodb/node_modules/mongodb-core/lib/cursor.js:154:5)
        at nextFunction (/usr/src/app/node_modules/mongodb/node_modules/mongodb-core/lib/cursor.js:675:5)
        at /usr/src/app/node_modules/mongodb/node_modules/mongodb-core/lib/cursor.js:588:7
        at /usr/src/app/node_modules/mongodb/lib/apm.js:543:13
        at queryCallback (/usr/src/app/node_modules/mongodb/node_modules/mongodb-core/lib/cursor.js:211:18)
        at Callbacks.emit (/usr/src/app/node_modules/mongodb/node_modules/mongodb-core/lib/topologies/server.js:116:3)
        at null.messageHandler (/usr/src/app/node_modules/mongodb/node_modules/mongodb-core/lib/topologies/server.js:282:23)
        at Socket.<anonymous> (/usr/src/app/node_modules/mongodb/node_modules/mongodb-core/lib/connection/connection.js:273:22)
        at emitOne (events.js:77:13)
        at Socket.emit (events.js:169:7)
        at readableAddChunk (_stream_readable.js:146:16)
        at Socket.Readable.push (_stream_readable.js:110:10)
        at TCP.onread (net.js:529:20)
    


  • @accalia does it work now? The change I made worked on the staging server but I don't want to lose my unresized avatar on live.


  • sockdevs

    @ben_lubar dunno. can you see my N7 logo?



  • @accalia refreshes yep. I'll submit a pull request.


  • sockdevs

    @ben_lubar i'll switch back to my new avatar then.





  • @ben_lubar No modifications to the tests directory. :cry:


  • sockdevs

    @ben_lubar said in Uploading new avatar crashes forum?:

    ..... looking at the diff in that PR i have no idea why that doen't have the same error.... if next isn't a function the replacement function should throw because you're treating it as a function there, and if it was a function then passing it directly to async should have worked....

    i must be missing something here.......



  • @accalia probably some reflection bullshit.


  • sockdevs

    @ben_lubar said in Uploading new avatar crashes forum?:

    @accalia probably some reflection bullshit.

    maybe, but unless you're playing shenanigans i'm not aware of with the v8 parser literally the only way to make next() valid is to have next be a function. you can't create an object and treat it as a function, you need to start with the function and treat it like an object...



  • @accalia next isn't the only function being called. The shenanigans are probably in async.series.



  • @accalia ah, I see what it was:

    async.series calls next with (err, resultsArray).

    That means the next function in the waterfall gets (resultsArray, next) instead of just (next).


  • sockdevs

    @ben_lubar said in Uploading new avatar crashes forum?:

    async.series calls next with (err, resultsArray).
    That means the next function in the waterfall gets (resultsArray, next) instead of just (next).

    oh.....

    yeah that would do it.



  • @accalia The async library is great, but it's confusing how it alters the conventions of callbacks WRT errors and data and callback functions.



  • @boomzilla C# has an async keyword, but I prefer Go's method of just writing normal blocking functions and making them concurrent using the go keyword.



  • @ben_lubar This module totally needs tests, both with and without plugin hooks.



  • @boomzilla did the test coverage patch ever get accepted?



  • @ben_lubar Nope.



  • @boomzilla ok, here's the code coverage

    I'm gonna go for a walk outside because it's a nice day. Does anyone know if Discourse measures code coverage?



  • @ben_lubar

    all files / src/user/ picture.js
    7.14% Statements 7/98 0% Branches 0/56 3.13% Functions 1/32 7.14% Lines 7/98
    

    Basically, no coverage (just the top level requires and exports setup).


  • sockdevs

    @ben_lubar said in Uploading new avatar crashes forum?:

    Does anyone know if Discourse measures code coverage?


  • Winner of the 2016 Presidential Election

    @accalia

    :doing_it_wrong:: our code covers several bikesheds. Of course we have test coverage, otherwise we wouldn't be able to have a tests-passed branch. You're no longer welcome here



  • @ben_lubar said in Uploading new avatar crashes forum?:

    Does anyone know if Discourse measures code coverage?

    There's a coverage dir in their .gitignore.

    Looking at the results of their CI for something that passed:

    Pending: (Failures listed here are expected and do not affect your suite's status)
      1) PrettyText Cooking allows html entities correctly
         # No reason given
         # ./spec/components/pretty_text_spec.rb:67
      2) Email::Styles basic formatter adds a max-width to images
         # No reason given
         # ./spec/components/email/styles_spec.rb:28
    
    ...there are more, but you get the idea
    

    That's...interesting. But I can't find any evidence of any actual coverage reports anywhere.


  • sockdevs

    @boomzilla

    Finished in 10 minutes 53 seconds (files took 10.9 seconds to load)
    

    that's a long build.....


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.