Repository navigation
chore: version bump typescript, uuid, ... , fix failing tests and typing issues - #646
kings1ay3r wants to merge 1 commit into
Conversation
- Update devDependencies in package.json - Fix TypeScript type errors in validation and parsers - Correct header serialization for objects - Resolve date and payload validation in test suites Signed-off-by: Kevin Mathew Sunny <tokevins@protonmail.com>
kings1ay3r
changed the title
|
@lance @lholmquist can you please take a look? |
|
@lance @lholmquist can you please take a look? |
| export class DateParser extends Parser { | ||
| parse(payload: string): string { | ||
| let date = new Date(Date.parse(payload)); | ||
| const date = new Date(Date.parse(payload)); |
There was a problem hiding this comment.
Creating Date object is not needed when the date is invalid.
Better simply check isNaN() right after Date.parse().
Source:
If dateString fails to be parsed as a valid date, NaN is returned.
Suggested version of this class:
export class DateParser extends Parser {
parse(payload: string): string {
const timestamp = Date.parse(payload);
return (
isNaN(timestamp)
? new Date()
: new Date(timestamp)
).toISOString();
}
}
| } else if (property !== CONSTANTS.DATA_ATTRIBUTE && property !== `${CONSTANTS.DATA_ATTRIBUTE}_base64`) { | ||
| headers[`${CONSTANTS.EXTENSIONS_PREFIX}${property}`] = value as string; | ||
| const headerName = `${CONSTANTS.EXTENSIONS_PREFIX}${property}`; | ||
| if (typeof value === "object") { |
There was a problem hiding this comment.
This might be a breaking change for projects that are using the wrong behavior (with "[object Object]").
Also note that typeof value === "object" is not a reliable check in JavaScript. For example, this check will destroy the error data:
- 🔴
JSON.stringify(new Error('useful message')) - 🟢
String(new Error('useful message'))
|
The only usage of overrides:
'uuid@8.3.2': '14.0.0' |
|
This pull request is stale because it has been open 30 days with no activity. |
|
@kings1ay3r I'm concerned about introducing a breaking change. |
|
@kings1ay3r I think this PR should be split up into 5 or more separate PRs. It would be great if the vulnerable dependencies could be upgraded without introducing new code. |
|
Correct. I have already taken care of this, mostly: Fixes many off the version bumps. @kings1ay3r either you can split each "breaking" change into its own PR or wait until I get to the refactoring |
|
@fhirter I'm working on it in my private branch -- ie split the commits into reasonable chunks vs the AI generated massive commits. It's a slog 🤣 One problem is that some of those upgrade break compatibility -- ie requires fixes |
The current implementation copies binary object and values into the header map as-is. This would produce incorrectly encoded HTTP headers, and the current behaviour relies on object coercion in axios. Instead we are explicitly serializing incompatible types to String. The CloudEvents type system has no object type, so accepting objects as extension values is an SDK convenience rather than spec behavior. In the long run, we should pivot to a spec compliant option, and throw an error for this case, and instead as the client to correctly set headers. Split from cloudevents#646. Signed-off-by: Shiva <shiv@shiv.me> Co-authored-by: Kevin Mathew Sunny <tokevins@protonmail.com> fix: update extension attribute handling for binary mode serialization
The current implementation copies binary object and values into the header map as-is. This would produce incorrectly encoded HTTP headers, and the current behaviour relies on object coercion in axios. Instead we are explicitly serializing incompatible types to String. The CloudEvents type system has no object type, so accepting objects as extension values is an SDK convenience rather than spec behavior. In the long run, we should pivot to a spec compliant option, and throw an error for this case, and instead as the client to correctly set headers. Split from cloudevents#646. Signed-off-by: Shiva <shiv@shiv.me> Co-authored-by: Kevin Mathew Sunny <tokevins@protonmail.com>
…ers (#666) * fix: serialize object-valued extension attributes in binary HTTP headers The current implementation copies binary object and values into the header map as-is. This would produce incorrectly encoded HTTP headers, and the current behaviour relies on object coercion in axios. Instead we are explicitly serializing incompatible types to String. The CloudEvents type system has no object type, so accepting objects as extension values is an SDK convenience rather than spec behavior. In the long run, we should pivot to a spec compliant option, and throw an error for this case, and instead as the client to correctly set headers. Split from #646. Signed-off-by: Shiva <shiv@shiv.me> Co-authored-by: Kevin Mathew Sunny <tokevins@protonmail.com> * fix: enhance plain object detection for binary-mode serialization Signed-off-by: Shiva <shiv@shiv.me> * fix: rename extensionHeaderValue to stringifyExtensionValue and update documentation for clarity Signed-off-by: Shiva <shiv@shiv.me> --------- Signed-off-by: Shiva <shiv@shiv.me> Co-authored-by: Kevin Mathew Sunny <tokevins@protonmail.com>
|
@kings1ay3r All of your changes + refactoring is now complete. I made sure your commits and authorship was ported over as much as possible as is. This final PR : #669 now resolves all the changes you proposed. Thanks very much for your effort. Once #669 is merged, we will be making a 10.1.0 launch. We can now close this PR. |
Proposed Changes
Description
Proper Header Serialization (src/message/http/headers.ts)
Previously, when an extension property was an object, it was improperly cast to a string via value as string. In JavaScript, this results in the exact string "[object Object]" being sent over the network, completely destroying the payload data. The fix properly uses JSON.stringify() when the header value is an object, preserving the data structure.
Type-Safe JSON Parsing (src/parsers.ts)
The JSONParser.parse() method was incorrectly typed to return a string, even though JSON.parse() returns an Object/Array. It was corrected to return unknown (which is standard for parsed JSON in TypeScript), preventing downstream typing assumptions.
Test Predictability (test/conformance/steps.ts & test/integration/emitter_factory_test.ts)
The conformance tests were blindly parsing missing dates, causing Invalid Date crashes if a time was missing. The fix safely checks if time exists before comparing date strings.
In emitter_factory_test.ts, the assertions now handle both pre-parsed objects and stringified JSON properties flexibly rather than blindly asserting deep equality on mismatched types.
Error Message Formatting (src/event/validation.ts)
The validation Error constructor was heavily relying on a dirty hack (// @ts-ignore) to coerce arrays of error objects into a single string. This was refactored to check Array.isArray(errors) and use a type-safe reduce loop to convert ErrorObject validation failures into a legible string.
Dependency Bumps (package.json)
DevDependencies like uuid, mocha, @cucumber/cucumber, axios, and typescript were suffering from legacy versioning that caused some of those implicit compilation errors you might have noticed. They were brought up to modern compatibility.
UUID package had a moderate vulnerability GHSA-w5hq-g745-h8pq
Fixes #647