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

Type error in Pinpoint::SendMessages response (possible regression) #422

Open
darrenf opened this issue Jan 17, 2022 · 1 comment
Open

Comments

@darrenf
Copy link

darrenf commented Jan 17, 2022

Hi Paws folks 👋

I've recently been attempting to use Paws::Pinpoint::SendMessages for Safari push notifications. The calls are successful (the notification turns up on the browser), but the client code barfs with this error:

Attribute (MessageResponse) does not pass the type constraint because: Validation failed for 'Paws::Pinpoint::MessageResponse'

Indeed, MessageResponse at this point is still a JSON string, it has not been marshalled into a MessageResponse object.

I had a look at the history of auto-lib/Paws/Pinpoint/SendMessages.pm and saw that the _returns type for this attribute was changed:

-  class_has _returns => (isa => 'Str', is => 'ro', default => 'Paws::Pinpoint::MessageResponse');
+  class_has _returns => (isa => 'Str', is => 'ro', default => 'Paws::Pinpoint::SendMessagesResponse');

And sure enough, if I reverse this change locally then the JSON gets marshalled correctly and the client no longer barfs.

The change was made by commit 14b1e06 (in PR #265) which I think might have been a merge/rebase/conflict resolution error responsible for the regression (and maybe others) - the diff is enormous, modifying 1700+ files! The commit date is Thu Jun 14 13:29:17 2018 +0000, which is a year before PR #335 was merged in 2019, fixing a lot of issues with Pinpoint. PR #265 was merged in March 2020.

As it happens, commit 8329499 (also in PR #265) has the same commit message (Add "uri other chars" test) and timestamp - but only modified 6 files including uri_avoid_chars.t and uri_other_chars.t. i.e. it's the commit that 14b1e06 claims to be! 😁

NB. I'm not 100% clear on whether a MessageResponse object is the correct return type, or if the bug is that it should be creating SendMessagesResponse object. I only know that my local patch has gotten me over the hump for now.

@darrenf
Copy link
Author

darrenf commented Jan 27, 2022

FYI, Paws::Pinpoint::GetApps is suffering similarly. I am currently working around these by redeclaring the attributes myself in my client code, like this:

Paws::Pinpoint::GetApps::class_has _returns => (
    isa     => 'Str',
    is      => 'ro',
    default => 'Paws::Pinpoint::ApplicationsResponse'
);

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

No branches or pull requests

1 participant