Uploading new avatar crashes forum?
-
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
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.
-
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...
-
@RaceProUK huh.... so the toaster went away, but the error's still there....
well then.....
-
Repro. Or at least the 502 aspect.
And pasting from clipboard broken again it seems.
One I was trying:
-
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:
-
@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.
-
@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
-
@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?
-
@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.
-
@boomzilla How long before they're broken again?...
-
@PJH That's a question for @ben_lubar. See also.
-
@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!
-
-
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.
-
@ben_lubar dunno. can you see my N7 logo?
-
@accalia refreshes yep. I'll submit a pull request.
-
@ben_lubar i'll switch back to my new avatar then.
-
-
@ben_lubar No modifications to the tests directory.
-
@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.
-
@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 havenext
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 inasync.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).
-
@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?
-
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).
-
@ben_lubar said in Uploading new avatar crashes forum?:
Does anyone know if Discourse measures code coverage?
-
: 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.
-
Finished in 10 minutes 53 seconds (files took 10.9 seconds to load)
that's a long build.....