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

UnsuccessfulResponse exception is not thrown when using preventUnsuccessfulResponse method #25

Open
phuclh opened this issue Aug 4, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@phuclh
Copy link

phuclh commented Aug 4, 2022

In the Spatie browsershot package, when using preventUnsuccessfulResponse method, it will throw UnsuccessfulResponse exception:

$browsershot->preventUnsuccessfulResponse($this->preventUnsuccessfulResponse);

But in this package, it throws Hammerstone\Sidecar\Exceptions\LambdaExecutionException, so it is hard to detect if a URL has HTTP error respond code like 404, 500, etc. Is it possible to throw UnsuccessfulResponse in Lambda environment?

Thank you!

@stefanzweifel
Copy link
Owner

I have to take a closer look at this.
The general problem with sidecar-browsershot seems to be, that users expect that Spatie's exceptions are thrown.

As mentioned in #19, maybe you can manually inspect the LambdaExectionException and in your app and act accordingly?

@stefanzweifel stefanzweifel added the bug Something isn't working label Aug 5, 2022
@phuclh
Copy link
Author

phuclh commented Aug 11, 2022

@stefanzweifel Yeah, that is what I am using now. I just took a quick look. Seems in the BrowsershotLambda, you threw all exception with this method:

CleanShot 2022-08-11 at 16 24 57@2x

While in the Spatie package, it checks by the exit code:
CleanShot 2022-08-11 at 16 26 11@2x

@stefanzweifel
Copy link
Owner

This seems to be more complicated than I thought.

Ideally, we could reuse the exit codes from Spatie's browser.js-code and do the same $exitCode check in BrowsershotLambda@callBrowser as Spatie does.

Right now, we do not differentiate between errors and body HTML here.
(Maybe I'm wrong and Sidecar already catches possible JavaScript/Node errors thrown in execSync. Then, result.toString() would always be the body HTML of a page. @aarondfrancis Do you have any insights about this?)

@phuclh You mention that you manually inspect the LambdaExecutionException. What exactly are you doing there right now? Do you re-throw the Spatie Exceptions? Are you able to share some code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants