Adjust the logging for the local endpoint implementation

Here we go. But it might make sense to change the texts a bit so that the reader (admin) better understands the context and meanings?

And should we add an option to turn the noise off perhaps? ;)
Or the other way around: "Debug mode" with a matching desciption?

What do you think? (Could also be done in a follow-up to get this done.)

Not quite what I envisioned! Since we now still have the issue of duplicate logging. I'll show you what I envisioned here.

This is what I had envisioned, please review!

And should we add an option to turn the noise off perhaps? ;)
Or the other way around: "Debug mode" with a matching description?

We could think of such a feature, I'd vote for having an option to turn off the noise. Not really a fan of the other way around, since I see the current error logging as necessary and something that you should deliberately have to turn off. But let's do that in another follow-up issue!

Something is wrong, wait a bit with the review.

Hm, interesting. Our solutions array always consists of 4 parts, instead of the expected 3 parts. I already reinstalled the module and it still consists of 4 parts:

I mean our original code: [$signature, $puzzle, $solutions] = $solutionArray would simply ignore the 4th array entry, but I am still wondering what it is used for...

@jurgenhaas, any idea?

We could think of such a feature, I'd vote for having an option to turn off the noise. Not really a fan of the other way around, since I see the current error logging as necessary and something that you should deliberately have to turn off. But let's do that in another follow-up issue!

Correct, if these are errors / notices the admin can do something about. If not, it should be off by default. And that was what I expected here.

@Anybody ok, I will create another follow-up issue for that. Let's keep this issue assigned to @jurgenhaas for the problem mentioned in #9 📌 Adjust the logging for the local endpoint implementation Active

I mean our original code: [$signature, $puzzle, $solutions] = $solutionArray would simply ignore the 4th array entry, but I am still wondering what it is used for...

That verify code comes from the friendly captcha folks, it's a clone of their own server. So, this is how they implemented that. I guess, that 4th argument is just relevant for that verify process.

Here is the link to the original code: https://github.com/FriendlyCaptcha/friendly-lite-server/blob/e93be67590a...

And indeed, they don't use that 4th one anywhere.

I'd suggest that we check for at least 3 parts in the array, not for exactly 3 parts.

@jurgenhaas thanks for clearing this up! 👍

Originally, I thought we should still check, that the array consists of exactly 4 parts, because in cases where it consists of only 3 parts the order of the array entries might be incorrect, leading to incorrect variable declarations, which might lead to other errors.
But one could think, that the friendlyCaptcha folks thought of that.
But since the original error with the 2 parameters instead of 4 puts me on the fence... Final thoughts @Anybody? Check for exactly 4 parts or at least 3?

I think we should check for the three required parameters ("at least three"), but add a comment about the 4th parameter:
"$diagnostics" which is unused. Perhaps linked to this discussion?

Alright! Adjusted the code accordingly. :)
Also used @ instead of % for the placeholder replacements, as the tags are being escaped on the recent log message:

@Grevil: Nice!

I left some comments. These are just meant to be placed as prefix in front of the message so it's easier for the admin to see what it's about!

Also I'm sure my comment form #10 applies. The admin can't do anything about it, so the debug mode should be opt-in!

Captcha validation failed! ID: "solution_malformed" Message: "Malformed solution: Expected at least 3 parts, got 4.".

LGTM now.

Here is the follow-up issue concerning #10: 📌 Create an option to turn off any logging from this module Active .

Nice! RTBC!

If @jurgenhaas has comments on the commit, let's solve them in a follow-up.

Thanks! Make sure, to always set the MR label manually. It doesn't get generated automatically according to the issue name / id anymore.

Automatically closed - issue fixed for 2 weeks with no activity.