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.

Sign up for GitHub

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

Jump to bottom

[notification hubs] Error on parsing XML in response when sending notification #25914

Closed
2 tasks done
hmaskat17 opened this issue May 18, 2023 · 9 comments
Closed
2 tasks done

[notification hubs] Error on parsing XML in response when sending notification #25914

hmaskat17 opened this issue May 18, 2023 · 9 comments
Assignees
@mpodwysocki
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. Notification Hub NotificationHubs Azure Notification Hubs

Comments

@hmaskat17
Copy link

  • Package Name: @azure/notification-hubs
  • Package Version: ^1.0.1
  • Operating system: Windows
  • nodejs
    • version: 18
  • typescript
    • version: 5

Describe the bug
When creating a notification with createFcmLegacyNotification and then sending it with sendNotification, a RestError is thrown stating that the response body could not be parsed: "Unexpected token < in JSON ..... while parsing the response body".

The response body is in XML format. Sending the notification automatically adds content-type value to application/json even though the response is XML and there is no way to change the content type's value.

To Reproduce
Steps to reproduce the behavior:

  1. Create a notification with createFcmLegacyNotification
  2. Send the notification with sendNotification

Expected behavior
The SDK should be able to parse the response correctly.
@github-actions github-actions bot added customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels May 18, 2023
@xirzec xirzec added bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. Notification Hub NotificationHubs Azure Notification Hubs and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels May 18, 2023
@github-actions github-actions bot removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label May 18, 2023
@mpodwysocki mpodwysocki mentioned this issue Jun 14, 2023
Merged
3 tasks
mpodwysocki added a commit that referenced this issue Jun 15, 2023
@mpodwysocki
[notification hubs] Update XML Error parsing for Issue #25914 (#26209)
79e943f 
### Packages impacted by this PR

- @azure/notification-hubs

### Issues associated with this PR

- #25914

### Describe the problem that is addressed by this PR

Updates code to fix the error code parsing for XML.

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?


### Are there test cases added in this PR? _(If not, why?)_


### Provide a list of related PRs _(if any)_


### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
- [x] Added a changelog (if necessary)
@mpodwysocki
Copy link
Member

@hmaskat17 does this continue to happen? I've published a new version which properly parses the error XML, however, this does not change the behavior where the response should be application/xml instead of application/json. If this does continue, contact the service team through the Azure Portal for assistance as that is on the service side, not the SDK side.

@mpodwysocki
Copy link
Member

@hmaskat17 can you run this sample to send to FCM Direct Send and let me know if it doesn't work for you?

@xirzec xirzec added the needs-author-feedback Workflow: More information is needed from author to address the issue. label Jul 31, 2023
@github-actions
Copy link

Hi @hmaskat17. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

@github-actions
Copy link

github-actions bot commented Aug 7, 2023

Hi @hmaskat17, we're sending this friendly reminder because we haven't heard back from you in 7 days. We need more information about this issue to help address it. Please be sure to give us your input. If we don't hear back from you within 14 days of this comment the issue will be automatically closed. Thank you!

@github-actions github-actions bot added the no-recent-activity There has been no recent activity on this issue. label Aug 7, 2023
@hmaskat17
Copy link
Author

@mpodwysocki I tested the sendNotification without updating the @azure/notification-hubs package and it does not give an error anymore and also seems to return a JSON object now. I also updated the package to 1.0.2 version and it also results in no error with a JSON object in response. So it seems that the response object was changed to JSON instead of XML.

@github-actions github-actions bot added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-author-feedback Workflow: More information is needed from author to address the issue. no-recent-activity There has been no recent activity on this issue. labels Aug 9, 2023
@xirzec xirzec added the issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. label Aug 14, 2023
@xirzec
Copy link
Member

xirzec commented Aug 14, 2023

@hmaskat17 am I correct in understanding the issue has been resolved? Please let us know if something is still amiss.

@github-actions github-actions bot removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Aug 14, 2023
@github-actions
Copy link

Hi @hmaskat17. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.

@hmaskat17
Copy link
Author

@xirzec Yes, the issue has been resolved.

@github-actions
Copy link

Hi @hmaskat17, since you haven’t asked that we /unresolve the issue, we’ll close this out. If you believe further discussion is needed, please add a comment /unresolve to reopen the issue.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@mpodwysocki mpodwysocki

Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. Notification Hub NotificationHubs Azure Notification Hubs
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@xirzec @mpodwysocki @hmaskat17