Skip to content

Conversation

@totten
Copy link
Member

@totten totten commented Dec 3, 2025

Overview

Sometimes, when logging an error, you want to generate a unique ID for that error -- so that you can tell the user about it. The user will share this error-ID with a sysadmin.

This pattern is a workflow convenience -- the sysadmin can search the error-logs more easily if there's a distinctive ID.

Before

Two instances of the pattern. Both generate IDs with random alphanumerics, but they use different formulas.

After

Same formula. Helper function can be called in other places (where we might introduce similar pattern).

Comment

Between the two formulas... random_bytes() is better than createRandom(). But 12 chars are better than 6 bytes. Allowing the full range of alphanumerics is nice (shorter IDs). Longer ID's make it harder for user+admin to communicate out-of-band (phone/SMS/chat/etc). Neither generator would be used for security-keys, but we're not doing that. We just need something that's unique enough to help the sysadmin filter through logs. Both are OK for the job.

(I kept the one from APIv4.)

@civibot
Copy link

civibot bot commented Dec 3, 2025

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...
  • If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
  • A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
  • A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
  • If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
  • Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
  • A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
  • To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
  • For more information about contributing, see CONTRIBUTING.md.
Quick links for reviewers...

➡️ Online demo of this PR 🔗

@civibot civibot bot added the master label Dec 3, 2025
@ufundo
Copy link
Contributor

ufundo commented Dec 4, 2025

I could quibble - two different ID formats allows the sage sysadmin to triage where the error is from without cross referencing to actual value in logs or whatever.

But r-run is fine for me.

@ufundo ufundo added the merge ready PR will be merged after a few days if there are no objections label Dec 4, 2025
@demeritcowboy demeritcowboy merged commit a71142c into civicrm:master Dec 6, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

master merge ready PR will be merged after a few days if there are no objections

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants