At least stuff loads!

[Onyx]

Wait...

[CreatedToDislikeThis]

@Onyx I think this happened to me too, making me post a long-irrelevant reply in the Deleted thread.

[lerrand]

FWIW, dunno if it's related, but I'm getting lots of these at random when entering topics or loading the main page:

(I mean the "Internal error" thing and the top-right infinispinner, not the migration notice, which also keeps popping up all the time/randomly, but I digress)

[ben_lubar]

Ok, it's crashed on this line an estimated 9999999 times in the last 12 hours.

NodeBB/src/middleware/render.js at master · NodeBB/NodeBB

Node.js based forum software built for the modern web - NodeBB/NodeBB

I've changed that line to check if req.next is defined first, but I still have to figure out why it wasn't defined in the first place.

[Kuro]

@ben_lubar said:

Ok, it's crashed on this line an estimated 9999999 times in the last 12 hours.

Wow, it's still better than Discourse?

Filed Under: How did it manage that?

[blakeyrat]

@ben_lubar Well I'm glad to see they did such a thorough QA cycle before merging in that code.

[cartman82]

@ben_lubar said:

I've changed that line to check if req.next is defined first, but I still have to figure out why it wasn't defined in the first place.

				defaultFn = function(err, str){
					if (err) {
						return req.next(err);
					}

					self.send(str);
				};

Why req.next(err) instead of just next(err)?

[ben_lubar]

@cartman82 ok, changed the code to just call next(err) instead of req.next && req.next(err). No crashes yet, so I'm gonna tell the NodeBB devs.

[cartman82]

@ben_lubar said:

@cartman82 ok, changed the code to just call next(err) instead of req.next && req.next(err). No crashes yet, so I'm gonna tell the NodeBB devs.

Whoa there cowboy. I only know that this looks strange from the pure express api POV. I didn't really look into the nodebb code. Maybe they have their own custom next function or something.

Better ask the nodebb devs first, before you realize this disables all the logging on the site or something.

[ben_lubar]

@cartman82 actually, it caused more errors to be logged. I'm pretty sure nothing in the entire program ever assigns to req.next.

[RaceProUK]

From a pure Node POV, calling next instead of req.next is the right way. Still, it's worth checking, just to be sure it's not going to cause more issues.

[ben_lubar]

@RaceProUK It's causing a bunch of Error: Can't set headers after they are sent., but none of those are crashing it so it's better.

[RaceProUK]

Hmm… that might be causing more subtle bugs to manifest.

What about

var f = req.next || next;
return f(err);

or similar? That is, if req.next is defined, use that, otherwise fall back to next?

Edit: Corrected as JS doesn't have a ?? operator; thanks @Onyx

[Onyx]

@RaceProUK Shouldn't that be

var f = req.next || next;
return f(err);