brefphp / bref

Serverless PHP on AWS Lambda
https://bref.sh
MIT License
3.1k stars 367 forks source link

POC: Adding specific sns and sqs exception when aws configration is invalid #1641

Closed robinlehrmann closed 1 year ago

robinlehrmann commented 1 year ago

This would help developers with configurations like SNS->Lambda when they use the wrong consumer.

GrahamCampbell commented 1 year ago

Is the stack trace not enough already?

robinlehrmann commented 1 year ago

Not sure, I thought it could be helpful @GrahamCampbell but up to you what do you think @mnapoli ? :)

mnapoli commented 1 year ago

I was writing this comment:


How about we keep the existing exception, but just improve the error message when possible? What I mean:

For example:

if (! is_array($record) || ! isset($record['eventSource']) || $record['eventSource'] !== 'aws:sqs') {
    throw new InvalidArgumentException($record['eventSource'] ?? '');
}

Then in InvalidArgumentException there could be a new optional argument:

public function __construct(string $eventSource = null) {
$message = 

Then I opened the code and realized we already have something similar, we actually embed the whole event in the exception message (throw new InvalidLambdaEvent('SQS', $this->event)).

So I'm sorry, but I think the current message is good enough and it's not worth adding special exceptions and error messages just for SQS and SNS when they are mixed together. This is a very specific scenario, adding specific code for this is not worth it I think. The generic exception already has enough information.

So in the end, I don't think it's worth changing anything actually 😬

robinlehrmann commented 1 year ago

ok, no worries @mnapoli :) Then feel free to close the issue as well