feat(user): return PictureURL on POST /user/info#121
Open
cesar-carlos wants to merge 7 commits into
Open
Conversation
CreateJID/ParseJID prefixes phone numbers with "+", which breaks the raw GetProfilePictureInfo IQ (Target=jid) and surfaces as "info query timed out" after ~75s on POST /user/avatar. Strip the prefix via CanonicalJID and ToNonAD before the request, matching the existing fix for reactions/typing/receipts. Closes evolution-foundation#76 Co-authored-by: Cursor <cursoragent@cursor.com>
Reviewer's GuideAdds a PictureURL field to POST /user/info responses and fixes JID canonicalization for usync and profile picture IQs by introducing a shared profile-picture helper and updating API docs. Sequence diagram for POST /user/info with PictureURL enrichmentsequenceDiagram
actor Client
participant ApiUserInfoHandler
participant UserService
participant WhatsmeowClient
Client->>ApiUserInfoHandler: POST /user/info
ApiUserInfoHandler->>UserService: GetUser
UserService->>UserService: utils.CanonicalJID + ToNonAD
UserService->>WhatsmeowClient: GetUserInfo(jids)
WhatsmeowClient-->>UserService: whatsmeowInfo
UserService->>UserService: fetchProfilePicture(client, jid, true, 25s)
UserService->>WhatsmeowClient: GetProfilePictureInfo(jid, preview)
WhatsmeowClient-->>UserService: ProfilePictureInfo(URL)
UserService-->>ApiUserInfoHandler: UserInfo{PictureID, PictureURL}
ApiUserInfoHandler-->>Client: 200 OK with PictureURL
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The 25s and 80s timeouts for profile picture lookups are hard-coded in different places; consider centralizing them as named constants to keep behavior consistent and easier to tune.
- The warning log on failed PictureURL enrichment in GetUser may become noisy under partial network issues; consider lowering this to debug or adding sampling if failures are expected to be common.
- The CanonicalJID().ToNonAD normalization is now applied in multiple flows; it might be helpful to wrap this into a single helper (e.g., canonicalUserJID/jidForProfilePicture) to avoid future divergences in JID handling.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The 25s and 80s timeouts for profile picture lookups are hard-coded in different places; consider centralizing them as named constants to keep behavior consistent and easier to tune.
- The warning log on failed PictureURL enrichment in GetUser may become noisy under partial network issues; consider lowering this to debug or adding sampling if failures are expected to be common.
- The CanonicalJID().ToNonAD normalization is now applied in multiple flows; it might be helpful to wrap this into a single helper (e.g., canonicalUserJID/jidForProfilePicture) to avoid future divergences in JID handling.
## Individual Comments
### Comment 1
<location path="pkg/user/service/user_service.go" line_range="332-341" />
<code_context>
+// fetchProfilePicture requests a profile picture URL for jid.
+// Never pass ExistingID here: when the picture is unchanged whatsmeow returns
+// nil with no error and no URL.
+func (u *userService) fetchProfilePicture(client *whatsmeow.Client, jid types.JID, preview bool, timeout time.Duration) (*types.ProfilePictureInfo, error) {
+ jid = utils.CanonicalJID(jid).ToNonAD()
+ ctx, cancel := context.WithTimeout(context.Background(), timeout)
+ defer cancel()
+
+ pic, err := client.GetProfilePictureInfo(ctx, jid, &whatsmeow.GetProfilePictureParams{
+ Preview: preview,
+ })
+ if err != nil {
+ return nil, fmt.Errorf("get profile picture for %s: %w", jid, err)
+ }
+ return pic, nil
+}
+
</code_context>
<issue_to_address>
**suggestion:** Consider centralizing timeout values instead of using magic numbers at call sites.
Callers currently pass literals like `25*time.Second` and `80*time.Second` into `timeout`. If these reflect distinct timeout policies, consider defining named constants or configuration options for them so they’re easier to understand and adjust without searching through all call sites.
Suggested implementation:
```golang
const (
profilePicturePreviewTimeout = 25 * time.Second
profilePictureFullTimeout = 80 * time.Second
)
// fetchProfilePicture requests a profile picture URL for jid.
// Never pass ExistingID here: when the picture is unchanged whatsmeow returns
// nil with no error and no URL.
```
```golang
pictureURL := ""
pic, picErr := u.fetchProfilePicture(client, jid, true, profilePicturePreviewTimeout)
if picErr != nil {
u.loggerWrapper.GetLogger(instance.Id).LogWarn("[%s] Failed to enrich PictureURL for %s: %v", instance.Id, jid, picErr)
} else if pic != nil {
pictureURL = pic.URL
}
```
1. Replace any other calls that currently pass `80*time.Second` as the timeout into `fetchProfilePicture` with `profilePictureFullTimeout` to align with the new constants.
2. If your project prefers configuration-driven timeouts, you can later wire these constants to configuration values (e.g., loaded from env/config) rather than hard-coded durations, while keeping the constant names as the single source of truth for callers.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
cesar-carlos
added a commit
to cesar-carlos/evolution-go
that referenced
this pull request
Jul 17, 2026
PictureURL on POST /user/info (PR evolution-foundation#121).
Author
|
Closing from the author side: these changes are already merged into our fork The commits remain available on the PR branch if maintainers still want to review/merge into |
Author
|
Reopened — keeping this PR available for upstream review/merge. |
Cap GetProfilePictureInfo at 8s using the request context so clients with ~12s read timeouts get HTTP 504 instead of a hung connection. Map WhatsApp rate-overlimit to HTTP 429. Co-authored-by: Cursor <cursoragent@cursor.com>
cesar-carlos
force-pushed
the
feat/user-info-picture-url
branch
from
July 18, 2026 15:03
e2f1627 to
077d662
Compare
8 tasks
Author
|
Updated after production avatar/info failure report (Chatwoot enrichment):
Please re-review the new commits on this branch. |
Prefer PN JID when the LID store has a mapping before the profile picture IQ. Replace fixed Sleep(2s) after StartInstance with a context-aware ready poll so avatar timeouts are not wasted on wait. Map context.Canceled to HTTP 504 like other request aborts. Co-authored-by: Cursor <cursoragent@cursor.com>
POST /user/info only returned PictureID and timed out on usync when the JID still had a "+" prefix. Canonicalize JIDs for GetUserInfo and enrich each user with a best-effort preview PictureURL (no ExistingID), sharing a small fetchProfilePicture helper with /user/avatar. Related to evolution-foundation#76 Co-authored-by: Cursor <cursoragent@cursor.com>
Use request context with a 10s usync deadline, shorten PictureURL enrich to 5s, map rate-overlimit/timeouts via writeUserWAError, and resolve @lid to PN via the LID store before usync when available. Co-authored-by: Cursor <cursoragent@cursor.com>
Prefer @lid then PN JID then digits; document HTTP 429/504 and fix avatar curl examples to use SERVER_PORT. Co-authored-by: Cursor <cursoragent@cursor.com>
Share a 5s wall-clock budget across all users after usync, skip enrich when PictureID is empty, and stop further enrich after rate-overlimit. Use context-aware client ready wait for GetUser as well. Co-authored-by: Cursor <cursoragent@cursor.com>
cesar-carlos
force-pushed
the
feat/user-info-picture-url
branch
from
July 18, 2026 15:12
077d662 to
e323425
Compare
Author
|
Follow-up improvements from review (rebased on updated #120):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
POST /user/infonever returned a profile picture URL (onlyPictureID) and, with a+-prefixed JID, also failed with usync"info query timed out"— same root cause as #76 / #120.What changed
GetUserInfo— strip+viaCanonicalJID+ToNonADbefore the usync query.PictureURLonUserInfo— best-effort preview URL afterGetUserInfo(5s timeout). Failures leavePictureURLempty and do not fail the whole/inforesponse.fetchProfilePicture(used by/user/avatarand/user/info). Does not passExistingID.rate-overlimit→ HTTP 429; IQ/context timeout → HTTP 504@lid→ PN viaStore.LIDs.GetPNForLIDbefore usync when available (P3 consistency)@lid→ PN JID → digits; document 429/504Response shape (additive, non-breaking)
{ "PictureID": "57501569", "PictureURL": "https://pps.whatsapp.net/v/..." }For full-resolution image, clients should still use
POST /user/avatarwithpreview: false.Related Issue
Related to #76
Depends on #120 (this branch includes those fix commits; rebase after #120 merges if needed)
Type of Change
Testing
PictureURLis additive; expected WA errors unchanged)Checklist
Additional Notes
Please review/merge #120 first if you prefer the bugfix alone; this PR stacks on that branch.