Skip to content

fix: headers in service inclusions should be optional#1188

Merged
derklaro merged 2 commits intonightlyfrom
fix/service-inclusions
Apr 12, 2023
Merged

fix: headers in service inclusions should be optional#1188
derklaro merged 2 commits intonightlyfrom
fix/service-inclusions

Conversation

@0utplay
Copy link
Member

@0utplay 0utplay commented Apr 11, 2023

Motivation

When including an inclusion that has no headers option set the following exception is thrown.
https://cdn.discordapp.com/attachments/818777626663321671/1095292788565868645/image.png

Modification

Made sure that we read an empty document if the headers option is not set.

Result

The header option is now optional as documented.

@0utplay 0utplay added v: 4.X This pull should be included in the 4.0 release in: node An issue/pull request releated to the node module code t: fix A pull request introducing a fix for a bug. labels Apr 11, 2023
@0utplay 0utplay added this to the 4.0.0-RC9 milestone Apr 11, 2023
@0utplay 0utplay requested a review from derklaro April 11, 2023 12:59
@0utplay 0utplay self-assigned this Apr 11, 2023
@0utplay 0utplay marked this pull request as ready for review April 11, 2023 12:59
var req = Unirest.get(inclusion.url());
// put the given http headers
var headers = inclusion.readProperty(ServiceRemoteInclusion.HEADERS);
var headers = inclusion.readPropertyOrDefault(ServiceRemoteInclusion.HEADERS, JsonDocument.emptyDocument());
Copy link
Member

Choose a reason for hiding this comment

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

Delayed until #1178, this should use Document.emptyDocument() after.

@derklaro derklaro merged commit eb2605f into nightly Apr 12, 2023
@derklaro derklaro deleted the fix/service-inclusions branch April 12, 2023 11:29
derklaro pushed a commit that referenced this pull request Apr 17, 2023
### Motivation
When including an inclusion that has no headers option set the following
exception is thrown.

https://cdn.discordapp.com/attachments/818777626663321671/1095292788565868645/image.png

### Modification
Made sure that we read an empty document if the headers option is not
set.

### Result
The header option is now optional as documented.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in: node An issue/pull request releated to the node module code t: fix A pull request introducing a fix for a bug. v: 4.X This pull should be included in the 4.0 release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants