FYI, minor bug with streamWithHeaders()

[starfishpatkhoo]

The new ->streamWithHeaders is good, but there is a bug... ?

If you do not have a "status" => 200 defined in the array, then there is an exception in Engine.php line 490: Undefined array key "status"

Documentation though says: optional status code, defaults to 200 .. Or maybe you meant the default HTTP header is 200, but that you must have a status?

It is not a big deal, just thought I would point it out.. ^_^

Flight version 3.8.1 ..

[fadrian06]

Thanks for notifying, it is already corrected, the patch will be published in a couple of hours

[starfishpatkhoo]

Thanks buddy! Like I said, not a big deal, just was confused myself because code and documentation didn't match.. LOL...

Now, it should be possible to do ->streamWithHeaders([]) and set all headers within your route function.. Feels neater that way..

[n0nag0n]

Now I'm just curious, what are you streaming? What's your use case?

[fadrian06]

Thanks buddy! Like I said, not a big deal, just was confused myself because code and documentation didn't match.. LOL...

Feel free to comment on any questions about the documentation, especially any issues you may have with the examples.

[starfishpatkhoo]

Now I'm just curious, what are you streaming? What's your use case?

Oh, well it's a small thing basically, we need to restrict access to a PDF by authorisation, so we check first, and if authorised, then send the binary data. If not, just 404 it or something. Or it could be a .pptx or whatever..

Other small things like checking if the PDF exists, and setting content-length, content-disposition file name, force no caching, force ob_end_clean(), etc, etc, all these are best done inside the function before we set/send the headers. Plus when I have time, I wanna add ranged bytes too etc.

TL;DR, lots of stuff to do, and I didn't want to put headers in the array in ->streamWithHeaders .. some headers are there, some are in the function, it gets a bit confusing.. So all headers in one place feels neater to me... Easier to find.

As I think about it, a special function Flight::routeStream() or Flight::routeMustAddYourOwnHeaders() where you were responsible for ALL headers and such might be more obvious... ha ha ha!

[n0nag0n]

As I think about it, a special function Flight::routeStream() or Flight::routeMustAddYourOwnHeaders() where you were responsible for ALL headers and such might be more obvious... ha ha ha!

Yeah it was taking a stab at common use cases and seeing where it went. Seems like the use cases have pivoted a bit and it might make more sense to force you to declare all the headers in your methods first by hand. Something for the future though! Thanks for explaining it for me! Tell your friends about Flight!

[starfishpatkhoo]

As I think about it, a special function Flight::routeStream() or Flight::routeMustAddYourOwnHeaders() where you were responsible for ALL headers and such might be more obvious... ha ha ha!

Yeah it was taking a stab at common use cases and seeing where it went. Seems like the use cases have pivoted a bit and it might make more sense to force you to declare all the headers in your methods first by hand. Something for the future though! Thanks for explaining it for me! Tell your friends about Flight!

Yep! that's what I'm doing right now (declaring all headers in the method).. some things, content-length, file name etc etc, just needs to be done after the route is declared. I can set a route("/*.pdf") and it works for all PDFs.

Like I said, it is not a big deal because everything works right now, so just wanted to share is all.. ^_^

[n0nag0n]

You bring up a good point. I made another PR to help simplify this even further so you could just call Flight::route()->stream(); and it'll work. https://github.com/flightphp/core/pull/575

[starfishpatkhoo]

Awesome! Thanks!

[n0nag0n]

Clarified and added the documentation as well. https://docs.flightphp.com/learn/routing#streaming

[n0nag0n]

Merged into master. Should be released soon.