Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Async functionality #89

Closed
g105b opened this issue Dec 29, 2020 · 3 comments · Fixed by #100
Closed

Replace Async functionality #89

g105b opened this issue Dec 29, 2020 · 3 comments · Fixed by #100

Comments

@g105b
Copy link
Member

g105b commented Dec 29, 2020

PHP.Gt/Async and PHP.Gt/Promise will reduce the dependency complexity.

@g105b g105b self-assigned this Sep 14, 2022
g105b added a commit that referenced this issue Sep 14, 2022
@g105b
Copy link
Member Author

g105b commented Sep 14, 2022

Currently the 1st "basic usage" example works fully, but if it's forced to decode JSON with a syntax error the promise rejection is eaten up silently... investigating next.

@g105b
Copy link
Member Author

g105b commented Jan 17, 2023

This is harder than I thought it would be.

@g105b g105b removed their assignment Jan 17, 2023
@g105b
Copy link
Member Author

g105b commented Jan 19, 2023

The entire codebase is greatly improved and tidy now, with more type safety, but this issue still remains as the only known bug.

public function json(int $depth = 512, int $options = 0):Promise {
$newDeferred = new Deferred();
$newPromise = $newDeferred->getPromise();
$deferredPromise = $this->deferred->getPromise();
$deferredPromise->then(function(string $resolvedValue)
use($newDeferred, $depth, $options) {
$builder = new JsonObjectBuilder($depth, $options);
try {
$json = $builder->fromJsonString($resolvedValue);
$newDeferred->resolve($json);
}
catch(JsonDecodeException $exception) {
$newDeferred->reject($exception);
}
});
return $newPromise;
}

This is where the issue lies, in the resolution of the JSON data. If the JSON data is invalid, like it is when used in example/01-basic-fetch.php, the exception is thrown rather than passed up the promise chain.

It's actually not a breaking issue, because it only represents internal failures (things like curl exceptions, timeouts or when $response->ok fails, are all still caught correctly). It might make sense to release the code in its currently improved state, then track this issue in a separate issue to return to later.

g105b added a commit that referenced this issue Jan 20, 2023
* wip: phpgt/promise implementation
for #89

* wip: refactor to gt promise model
still need to handle rejection exceptions

* feature: add badly formed json for testing

* ci: update workflows

* build: update dependencies

* docs: update example to match new functionality

* wip: sign off Http class

* wip: use master branch of async

* wip: fix existing tests

* feature: implement proper blob response type

* wip: tidy request resolver

* wip: tidy curl opt builder

* wip: tidy http class

* stan: improve static analysis over core classes

* test: improve types for native curl handle

* stan: remove unused import

* wip: document BodyResponse internal rejection

* wip: remove alternative method of throwing internal exception

* docs: update examples
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 a pull request may close this issue.

1 participant