Feedback

[razshare]

Chitchat, brainstorming and experiences, all welcome here.

[eurosat7]

This thing is massive and it looks quite strong. Thanks for sharing! :)

I started looking at the docs.

docs/1.router.md

I have a question for understanding your docs/1.router.md

You try to create a server. If that does not work you yield an error. This part is fine. But why is you main() even trying to then define a route afterwards? You will never be able to run it anyways. For me this makes no sense at all. Not being able to create a server is a fatal error situation. If you want to stay true to your logic I would not just yield the error but also return void to get out of the error generator.

Your error management mechanic is a really cool idea. We do something alike - sometimes. In this case I think throwing a fatal error could be a better option... But I haven't looked at the outer context yet.

Do you lack type declaration here?

It feels like boilerplate having to write:

    // 1. Open file
    $fileAttempt = File::open('file.txt');
    if ($fileAttempt->error){
      return error($fileAttempt->error);
    }

When calling a main() you could expect a fatal error to be thrown and catch it outside (via middleware?). This would make writing code much slimmer and prevent to continue after an unrecoverable situation. But I think you prefer each route to be able to do its own thing in handling situations without having to throw.

Option Attribute

The idea of the Option Attribute is sexy. :D

EnvironmentService

EnvironmentService::setFileName can be removed if you extend your constructor to allow a second parameter.

    public function __construct(
        private readonly LoggerInterface $logger,
        private readonly string $fileName = './env.yaml',
    ) {
    }

You even could go hardcore ("defaults are stored as null for maximum flexability") and use null (and allow null) as default and set the default in line 69:

   $fileName = $this->fileName ?? './env.yaml';

But that reduces readability in this case.

composer scripts

I would love to see you using a Makefile instead of composer scripts for readability.

---

I will continue reading the code.

[razshare]

Hi @eurosat7 ,
thank you for the feedback, it is much appreciated.

I'll try answer your questions.

But why is you main() even trying to then define a route afterwards?

I'm assuming you're speaking of this part

<?php
use function CatPaw\Core\anyError;
use CatPaw\Core\Unsafe;
use CatPaw\Web\Server;

function main(): Unsafe {
    return anyError(function() {
        $server = Server::create()->try($error)
        or yield $error;

        $server->start()->await()->try($error)
        or yield $error;
    });
}

This is because the anyError() function consumes your generator step by step and will stop immediately when it encounters an error.

Say the server creation fails for some reason, the statement below will never be executed, instead the anyError() function will stop execution immediately.

When you call ->try($error), you're unwrapping the Unsafe object, if there's any error within the object then $error is given the value of the error and the whole expression returns null, which means the second part of your statement, the or yield $error is instead triggered, yielding an error to anyError(), which stops execution.

return anyError(function() {
        $server = Server::create()->try($error)
        or yield $error;  // <=== if this triggers ...

        $server->start()->await()->try($error) // ... this will not execute
        or yield $error;
    });
}

The anyError() itself returns a new Unsafe, so if you want you could manage that error also, like this

<?php
use function CatPaw\Core\anyError;
use CatPaw\Core\Unsafe;
use CatPaw\Web\Server;

function main(): Unsafe {
    $result = anyError(function() {
        $server = Server::create()->try($error)
        or yield $error;

        $server->start()->await()->try($error)
        or yield $error;
    });
    if($result->error){
        echo "something went wrong...\n";
        die();
    }
}

But there's no need, that use case is already being handled here
https://github.com/tncrazvan/catpaw/blob/aa0fd66cd1087f1fd36752bc9a84f77600abfc48/src/lib/Core/Bootstrap.php#L162-L166

When calling a main() you could expect a fatal error to be thrown and catch it outside (via middleware?).

That is also handled already

https://github.com/tncrazvan/catpaw/blob/aa0fd66cd1087f1fd36752bc9a84f77600abfc48/src/lib/Core/Bootstrap.php#L169-L171

which is what you're suggesting, except I'm not dealing with exceptions, the whole point is to get rid of exceptions and not interrupt the control flow.

Routes, however can only return ResponseModifier objects, which you can create with success(), failure() and websocket().

Note

Reading the docs I just realized they were wrong saying handlers could return primitives and other things, they cannot, they must return ResponseModifier.
I just corrected the docs.

This would make writing code much slimmer and prevent to continue after an unrecoverable situation.

That's why anyError() exists, to avoid having to write

if ($fileAttempt->error){
    return error($fileAttempt->error);
}

all the time.

Though the if ($fileAttempt->error) form is still valuable to me and arguably the best part about Unsafe.
The fact that your error is a result opens many doors (as you can see at the bottom of this post, from my match($error::class) example).

Favor reading code over writing code

I've tried many variations and many different approaches, I believe this to be the most concise way to do it right now in php.
My approach is inspired very much by Zig's approach, which you can find here.

With the exception (no pun intended) that in php we cannot return from the whole context like you can in Zig.
So while this is possible in Zig

const parseU64 = @import("error_union_parsing_u64.zig").parseU64;

fn doAThing(str: []u8) !void {
    const number = parseU64(str, 10) catch |err| return err;
    _ = number; // ...
}

Note the return err; at the end, that returns a result for the doAThing function, we can't do that in php as far as I know.
But we can replicate it with Generators, hence; yield $error.

So basically when you yield any error inside anyError() it means you stop the execution of that given anyError() call.

The idea of the Option Attribute is sexy. :D

Option is a good idea and you can still use them, it doesn't conflict in any way with anything, but Unsafe has the added advantage it forces you to specify an error.

Generally speaking I don't trust my lazy ass at 4 AM, so I force myself to manage errors.
And generally speaking I avoid nulls in my programs every time I get the chance.

Although Unsafe coincidentally manages null values, my goal with unsafe is to manage errors.

For example, with Unsafe you can match on different types of errors like this (also inspired by Zig):

function someFunction(){
    $result = someUnsafeFunction()->try($error) or match($error::class){
        MyCustomError::class => "some fallback value",
        MyOtherCustomError::class => "some other fallback value",
    };
}

Not only it doesn't break control flow, you get to use pattern matching and it even crashes the program (which is what I want to happen unless I'm specifically managing an error myself) if you don't manage a given error use case, because match cases must be exhaustive in php.

So if someUnsafeFunction() produces SomeCustomUnknownError let's say, you'll get a php error, because you're not managing the use case.

[razshare]

@eurosat7 this is the whole code for anyError() btw, notice the return statements.

https://github.com/tncrazvan/catpaw/blob/aa0fd66cd1087f1fd36752bc9a84f77600abfc48/src/scripts/Core/functions.php#L218-L270

[razshare]

@eurosat7

I would love to see you using a Makefile instead of composer scripts for readability.

Noted!

[eurosat7]

Ah I see. anyError() stops with first yield. Now I have a new connection in my brain: "Even if I have a Generator - I do not have to run every yield." - Never had thought about that before. Thanks. :D