Skip to content

Added debug logging of MediaWiki requests and responses#101

Merged
Xymph merged 4 commits intohamstar:masterfrom
Xymph:101-debug-logging
Jul 1, 2021
Merged

Added debug logging of MediaWiki requests and responses#101
Xymph merged 4 commits intohamstar:masterfrom
Xymph:101-debug-logging

Conversation

@Xymph
Copy link
Copy Markdown
Collaborator

@Xymph Xymph commented Jun 28, 2021

During my own development and testing of Wikimate and my own client scripts, I found these request/response debug logs rather useful, so adding them to the repo.

I can imagine that this is too much logging in some situations and that we'd like debug levels, but that would require further changes and possibly an API-breaking change as $debugMode is currently a boolean.

Would anyone have additional thoughts on this? @waldyrious perhaps?

Also applying some comment improvements that aren't worth a separate branch.

@Xymph Xymph merged commit a09ff66 into hamstar:master Jul 1, 2021
@waldyrious
Copy link
Copy Markdown
Collaborator

Sorry I couldn't review this on time. In any case, I did take a look now, and everything looks good on my end. Nice commit separation, as always :)

@Xymph Xymph deleted the 101-debug-logging branch July 1, 2021 10:45
Comment thread Wikimate.php
This was referenced Jul 4, 2021
@Xymph Xymph mentioned this pull request Jul 9, 2021
@Xymph
Copy link
Copy Markdown
Collaborator Author

Xymph commented Aug 26, 2021

The question in the initial description about possibly changing $debugMode from boolean into integer kinda snowed under here. I can imagine some users preferring to select how much debug logging is generated, but I'm not yet clear on what characteristics to select that.

In my own practice of use debug, it is only temporary to figure out some problem, and then I don't mind getting to see all requests & responses of all API calls, and just skip to the one(s) I need to analyze.

Any thoughts, @waldyrious ?

@waldyrious
Copy link
Copy Markdown
Collaborator

waldyrious commented Aug 26, 2021

Regardless of preference, I believe it's pretty standard to group log messages into debug, info, warn and error categories, which would suggest replacing $debugMode with something like $logLevel.

As for preference, I've had to deal with overly verbose programs and personally would rather have the ability to request clearer logs in the first place, than having to filter information out myself.

Since those who don't mind verbose logs can simply use a more verbose logging level (if those are available), but the converse isn't true if they aren't, I think it would be a net positive to err on the side of flexibility here.

@Xymph
Copy link
Copy Markdown
Collaborator Author

Xymph commented Aug 28, 2021

I've made good progress on #113 and in retesting some client scripts with debug logging, I've grown to feel there's no real benefit to debug levels anymore. My original thinking there was related to token requests vs. query/other requests, because each edit/delete/upload refetched the CSRF token. Level 1 (e.g.) would then keep token requests out of the logging, and level 2 include them. But that no longer happens since #115. In the current situation, I can't think of a sensible division of what to log at which level.

Now with centralized logging of GET requests to and GET/POST responses from the API, I find to analyze a problem I simply need to check all requests in a flow. And analyzable problems are more often in the Wikimate classes than in client scripts. So unless and until other users express more refined needs for debug logging, I think it's better to keep this Wikimate-simple. 😃

Btw, the info/warning/error grouping doesn't apply to this library: it doesn't output any informational messages, and errors/warnings (from the API or Wikimate itself) are returned via getError() or exceptions.

@waldyrious
Copy link
Copy Markdown
Collaborator

Sounds good to me. Simple is good :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants