nextcloud / nextcloud/server

`GuzzlePromiseAdapter::then()` breaks promise chaining by returning `$this`

Open
#59,178 4 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

0. Needs triage 34-feedback bug
Dominant language
PHP
Stars
36.9k
Forks
5.2k
Avg merge
2d 3h
Merged PRs (30d)
713

Description

I believe this is a bug; ran across while poking around at other matters.

OC\Http\Client\GuzzlePromiseAdapter::then() calls the underlying Guzzle promise’s then(...), but discards the returned promise and returns $this instead.

This breaks normal promise chaining semantics.

https://github.com/nextcloud/server/blob/2a5e57d2f4d8c6365a0a16c38837d97d414e0002/lib/private/Http/Client/GuzzlePromiseAdapter.php#L34-L66

The current implementation also contradicts the IPromise::then() docs, which say it returns a new promise resolving to the handler’s return value.

For what it's worth, app_api is currently a consumer...

https://github.com/nextcloud/app_api/blob/c9364a51c2491c0aadc2d200b50d40e4f5c3a037/lib/Service/AppAPIService.php#L131-L149

Also could use some unit tests for async contract behavior...

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in lib/private/Http/Client/GuzzlePromiseAdapter.php at GuzzlePromiseAdapter::then(), then compare its behavior with the IPromise::then() documentation and the underlying Guzzle promise. Add unit coverage for the async contract, including chaining and handler return values, and verify the app_api usage in lib/Service/AppAPIService.php remains compatible.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.