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

Validate all fields on an Event can be altered in an OnErrorCallback #2010

Merged
merged 1 commit into from
Apr 16, 2024
Merged

Validate all fields on an Event can be altered in an OnErrorCallback #2010

merged 1 commit into from
Apr 16, 2024

Conversation

YYChen01988
Copy link
Contributor

Goal

Validates all fields on an event can be altered in an OnErrorCallback.

Design

A new scenario to change all the changeable fields in an event and validated it in an end to end test

Testing

Introduce new end to end test

@YYChen01988 YYChen01988 requested a review from lemnik April 11, 2024 08:28
@bugsnagbot
Copy link
Collaborator

bugsnagbot commented Apr 11, 2024 •
edited
Loading

Android notifier sizes

Format Size impact of Bugsnag (kB) Size impact of Bugsnag when Minified (kB)
APK 1844.5 1668.59
arm64_v8a 626.95 450.82
armeabi_v7a 561.42 385.29
x86 704.75 524.53
x86_64 671.99 495.86

Generated by 🚫 Danger

@YYChen01988 YYChen01988 force-pushed the PLAT-5883/validates_fields_on_an_error/session_can_be_altered_in_an_OnErrorCallback branch 4 times, most recently from 384e915 to f7ccb2c Compare April 11, 2024 09:26
@lemnik lemnik changed the title Validates fields on an event can be altered in an OnErrorCallback Validate all fields on an event can be altered in an OnErrorCallback Apr 11, 2024
@lemnik lemnik changed the title Validate all fields on an event can be altered in an OnErrorCallback Validate all fields on an Event can be altered in an OnErrorCallback Apr 11, 2024
@YYChen01988 YYChen01988 force-pushed the PLAT-5883/validates_fields_on_an_error/session_can_be_altered_in_an_OnErrorCallback branch 2 times, most recently from a6e836b to 7481a34 Compare April 11, 2024 13:04
lemnik
lemnik reviewed Apr 12, 2024
View reviewed changes
Copy link
Contributor

@lemnik lemnik left a comment

Choose a reason for hiding this comment

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

I think it would be good to not clear out all of the data added before the callback, so that we check that the original data isn't accidentally manipulated

...java/com/bugsnag/android/mazerunner/scenarios/UnhandledExceptionEventDetailChangeScenario.kt Outdated Show resolved Hide resolved
...java/com/bugsnag/android/mazerunner/scenarios/UnhandledExceptionEventDetailChangeScenario.kt Outdated Show resolved Hide resolved
features/full_tests/error_callback_alters_fields.feature Outdated Show resolved Hide resolved
features/full_tests/error_callback_alters_fields.feature Show resolved Hide resolved
@YYChen01988 YYChen01988 force-pushed the PLAT-5883/validates_fields_on_an_error/session_can_be_altered_in_an_OnErrorCallback branch 2 times, most recently from 758e73a to d58a29a Compare April 12, 2024 09:12
@YYChen01988 YYChen01988 requested a review from lemnik April 12, 2024 09:12
lemnik
lemnik approved these changes Apr 16, 2024
View reviewed changes
Copy link
Contributor

@lemnik lemnik left a comment

Choose a reason for hiding this comment

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

LGTM

@YYChen01988 YYChen01988 force-pushed the PLAT-5883/validates_fields_on_an_error/session_can_be_altered_in_an_OnErrorCallback branch from d58a29a to c1c6561 Compare April 16, 2024 08:40
@YYChen01988 YYChen01988 merged commit 1c44dd1 into next Apr 16, 2024
26 checks passed
@YYChen01988 YYChen01988 deleted the PLAT-5883/validates_fields_on_an_error/session_can_be_altered_in_an_OnErrorCallback branch April 16, 2024 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lemnik lemnik lemnik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@YYChen01988 @bugsnagbot @lemnik