Skip to content

Fix errors not matching PurchasesError - #579

Open
vegaro wants to merge 6 commits into
mainfrom
fix/error-unwrapping-issue-302
Open

Fix errors not matching PurchasesError#579
vegaro wants to merge 6 commits into
mainfrom
fix/error-unwrapping-issue-302

Conversation

@vegaro

@vegaro vegaro commented Oct 29, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

We have this long standing issue #302 that reported that our errors don't match the typescript interface, particularly in iOS.

The main reason is that in Android, Capacitor automatically puts whatever is passed as the third parameter of the reject function in a data property in the error. In iOS, we were not even passing the user info, so the errors looked like this:

Before the change

in Android:

{
  "code": "11",
  "data": {
    "code": 11,
    "message": "There was a credentials issue. Check the underlying error for more details.",
    "readableErrorCode": "InvalidCredentialsError",
    "readable_error_code": "InvalidCredentialsError",
    "underlyingErrorMessage": "Invalid API Key."
  }
}

in iOS:

{
  "code":"11",
  "errorMessage":"There was a credentials issue. Check the underlying error for more details. Invalid API Key.",
  "message":"There was a credentials issue. Check the underlying error for more details. Invalid API Key."
}

After the change

in Android:

{
  "code": "11",
  "data": {
    "code": 11,
    "message": "There was a credentials issue. Check the underlying error for more details.",
    "readableErrorCode": "InvalidCredentialsError",
    "readable_error_code": "InvalidCredentialsError",
    "underlyingErrorMessage": "Invalid API Key."
  },
  "message": "There was a credentials issue. Check the underlying error for more details.",
  "readableErrorCode": "InvalidCredentialsError",
  "userInfo": {
    "readableErrorCode": "InvalidCredentialsError"
  },
  "underlyingErrorMessage": "Invalid API Key.",
  "userCancelled": null
}

in iOS:

{
  "code": "11",
  "data": {
    "message": "There was a credentials issue. Check the underlying error for more details. Invalid API Key.",
    "readable_error_code": "INVALID_CREDENTIALS",
    "code": 11,
    "readableErrorCode": "INVALID_CREDENTIALS",
    "underlyingErrorMessage": "Invalid API Key."
  },
  "errorMessage": "There was a credentials issue. Check the underlying error for more details. Invalid API Key.",
  "message": "There was a credentials issue. Check the underlying error for more details. Invalid API Key.",
  "readableErrorCode": "INVALID_CREDENTIALS",
  "userInfo": {
    "readableErrorCode": "INVALID_CREDENTIALS"
  },
  "underlyingErrorMessage": "Invalid API Key.",
  "userCancelled": null
}

vegaro added 3 commits October 29, 2025 14:11
@vegaro
Fix error structure mismatch on iOS and Android (issue #302)
1b96d50 
- Fix iOS to use 4-parameter reject() with error.info instead of error.error
- Add TypeScript proxy layer to unwrap Capacitor's data field
- Ensure error properties are accessible at top level on both platforms
- Add logging to verify error unwrapping is working
- Update README to reflect correct error structure

Fixes: #302
@vegaro
fix error structure
a1d0f19
@vegaro
clean up
27eef48
@vegaro
vegaro requested a review from a team as a code owner October 29, 2025 14:54
@vegaro vegaro added the pr:fix label Oct 29, 2025
ajpallares
ajpallares reviewed Oct 29, 2025
View reviewed changes

@ajpallares ajpallares left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense!

Should we add some unit tests for this?

Also, I'll leave approval to someone with more ts knowledge 🙏

tonidero
tonidero approved these changes Oct 30, 2025
View reviewed changes

@tonidero tonidero left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question, but if we've tested it with most functions on the SDK in both Android and iOS, it does make sense. Thank you so much for handling this!!

Comment thread src/index.ts
target: any,
methodName: string | symbol
): T {
const wrappedFn = function(this: any, ...args: any[]) {

@tonidero tonidero Oct 30, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we could apply a fix here so that, if any parameters are of type Vue.js reference (or maybe use something as a proxy, like the presence of a toRaw function, use that. In order to avoid these issues we've had with Vue.js reactivity... But in any case, that can come in a separate PR.

@vegaro vegaro Nov 3, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I have another branch for that, but since I am not quite sure how to test it. It's in another branch. Just opened the draft PR #583

Comment thread src/index.ts
const value = Reflect.get(target, prop, receiver);

if (typeof value === 'function') {
if (typeof prop === 'number') {

@tonidero tonidero Oct 30, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I'm wondering is this ever the case? If i'm not wrong here, prop is the function name right?

@vegaro vegaro Nov 3, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can occur if a function gets called with index access [0], but I will double check

@vegaro vegaro Nov 3, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might be able to skip this tbh

@vegaro

vegaro commented Jan 12, 2026

Copy link
Copy Markdown
Member Author

I should check this still works fine with Capacitor 8 before merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ajpallares ajpallares ajpallares left review comments
@tonidero tonidero tonidero approved these changes

Assignees

No one assigned

Labels

pr:fix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vegaro @tonidero @ajpallares