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

fix: support for raw data #89

Merged
merged 22 commits into from
Feb 6, 2025
Merged

fix: support for raw data #89

merged 22 commits into from
Feb 6, 2025

Conversation

ZmRZjFuDsDiR4qi5801F
Copy link
Contributor

Bug 981245: node-red-contrib-ctrlx-automation: No support for RAW data

@ZmRZjFuDsDiR4qi5801F
Copy link
Contributor Author

@krauskopf : "skipped unittest 'ctrlx-datalayer-subscribe: Error Handling', please adapt to new behaviour and enable again.

@ZmRZjFuDsDiR4qi5801F
Copy link
Contributor Author

ZmRZjFuDsDiR4qi5801F commented Jan 23, 2025

changed: the nodes now return the data as-it-is with type 'raw', if not valid JSON (server failure or raw data)

@krauskopf krauskopf self-requested a review January 23, 2025 15:43
@krauskopf
Copy link
Contributor

I am missing a comment in the changelog of the readme.md

@ZmRZjFuDsDiR4qi5801F
Copy link
Contributor Author

@krauskopf: using default VS Code formatting, so please adopt or "hide white space"

@krauskopf
Copy link
Contributor

@krauskopf: using default VS Code formatting, so please adopt or "hide white space"

Can you make your VSCode config to consider the .editorconfig settings?
https://github.com/boschrexroth/node-red-contrib-ctrlx-automation/blob/master/.editorconfig

@ZmRZjFuDsDiR4qi5801F
Copy link
Contributor Author

ZmRZjFuDsDiR4qi5801F commented Jan 27, 2025

@krauskopf : It's not the .editorconfig, it's your used formatting extension which differs from current default one and Prettier, maybe you're using a deprecated one. I tried to restore the previous formatting, but please think about to update. Thank you.

Copy link
Contributor

@krauskopf krauskopf left a comment

Choose a reason for hiding this comment

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

Looks good to me. Ready for merge.

* @returns {object} - The parsed javascript object.
* @memberof CtrlxDatalayer
* @throws {SyntaxError} On invalid JSON objects.
*/
static _parseData(data) {

// We expect JSON.parse to fail here for invalid JSON
Copy link
Contributor

Choose a reason for hiding this comment

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

// It may happen, that JSON.parse may fail here for invalid JSON. But we catch this in the calling function.


// We expect 200 on success
if (res.statusCode !== 200) {
callback(CtrlxProblemError.fromHttpResponse(res, data));
return;
}

// Try to parse the data
// Try to parse the data, else we return the binary buffer (raw)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we use the "content-type" in the response header to distinguish between binary data ("octet-stream") and JSON data ("application/json") in the first place?
Returning raw/data if json parsing fails could be a fallback strategy, but actually it is an error of the server to send wrong encoded data.

Pseudo-Code:

if (respons.header("content-type") == "application/json") {
  payload = CtrlxDatalayer._parseData(data);
} else {
 payload = {
            type: 'raw',
            value: buffer,
          }
}

@krauskopf krauskopf merged commit e11b6ee into master Feb 6, 2025
3 checks passed
@krauskopf krauskopf deleted the bugfix/rawdata branch February 6, 2025 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants