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

Handle IndexError when reading msg files #671

Merged
merged 11 commits into from
Jun 19, 2024
Merged

Handle IndexError when reading msg files #671

merged 11 commits into from
Jun 19, 2024

Conversation

viktorerlingsson
Copy link
Member

@viktorerlingsson viktorerlingsson commented May 7, 2024 •
edited
Loading

WHAT is this pull request doing?

Handles a case where a msg file size is bigger than expected. Tries to move on to the next segment if possible.

fixes: #669

HOW can this pull request be tested?

Run spec

carlhoerberg
carlhoerberg reviewed May 7, 2024
View reviewed changes
Copy link
Member

@carlhoerberg carlhoerberg left a comment

Choose a reason for hiding this comment

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

Let's be more generic than just BytesMessage::MIN_BYTESIZE

@viktorerlingsson viktorerlingsson changed the title msg file size does not match IndexError when reading msg file Jun 19, 2024
@viktorerlingsson viktorerlingsson changed the title IndexError when reading msg file Handle IndexError when reading msg file Jun 19, 2024
@viktorerlingsson viktorerlingsson changed the title Handle IndexError when reading msg file Handle IndexError when reading msg files Jun 19, 2024
@viktorerlingsson viktorerlingsson force-pushed the 669-msg_size_does_not_match branch from 15d063d to 400106b Compare June 19, 2024 15:21
@viktorerlingsson viktorerlingsson marked this pull request as ready for review June 19, 2024 15:24
@viktorerlingsson viktorerlingsson force-pushed the 669-msg_size_does_not_match branch from 400106b to 11b32f4 Compare June 19, 2024 22:40
@viktorerlingsson viktorerlingsson merged commit 1db4c6c into main Jun 19, 2024
23 of 25 checks passed
@viktorerlingsson viktorerlingsson deleted the 669-msg_size_does_not_match branch June 19, 2024 22:44
spuun pushed a commit that referenced this pull request Aug 26, 2024
@viktorerlingsson @spuun
Handle IndexError when reading msg files (#671)
2bfe77e 
Don't close queues if only msg file size is incorrect
@spuun spuun mentioned this pull request Aug 26, 2024
Closed
viktorerlingsson added a commit that referenced this pull request Sep 20, 2024
@viktorerlingsson
Handle IndexError when reading msg files (#671)
d45aab5 
Don't close queues if only msg file size is incorrect
@viktorerlingsson viktorerlingsson mentioned this pull request Dec 3, 2024
Merged
viktorerlingsson added a commit that referenced this pull request Dec 4, 2024
@viktorerlingsson
Rescue IndexError in first? and move on to next segment (#864)
d4db0f5 
If a msg file is a few bytes larger than expected, first? will throw an IndexError and close the queue. This changes that behavior to instead rescue that IndexError and try to move on to the next segment file. Same fix as for shift? in #671

fixes #669 (again 🙂)
kickster97 pushed a commit that referenced this pull request Dec 16, 2024
@viktorerlingsson @kickster97
Rescue IndexError in first? and move on to next segment (#864)
fbff0b0 
If a msg file is a few bytes larger than expected, first? will throw an IndexError and close the queue. This changes that behavior to instead rescue that IndexError and try to move on to the next segment file. Same fix as for shift? in #671

fixes #669 (again 🙂)
kickster97 pushed a commit that referenced this pull request Dec 18, 2024
@viktorerlingsson @kickster97
Rescue IndexError in first? and move on to next segment (#864)
5b8332a 
If a msg file is a few bytes larger than expected, first? will throw an IndexError and close the queue. This changes that behavior to instead rescue that IndexError and try to move on to the next segment file. Same fix as for shift? in #671

fixes #669 (again 🙂)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlhoerberg carlhoerberg carlhoerberg approved these changes

@kickster97 kickster97 Awaiting requested review from kickster97

@snichme snichme Awaiting requested review from snichme

@spuun spuun Awaiting requested review from spuun

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

Successfully merging this pull request may close these issues.

Some of the queues are closed
2 participants
@viktorerlingsson @carlhoerberg